Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement convenience factory methods for Async[Throwing]Stream #62968

Merged
merged 2 commits into from
Mar 5, 2023

Conversation

FranzBusch
Copy link
Member

This is the implementation for swiftlang/swift-evolution#1824

@FranzBusch FranzBusch force-pushed the fb-async-stream-factories branch from 2d4155d to 4326a34 Compare January 11, 2023 13:07
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM AFAICS! Pending evolution approval.

Need to decide what swift version this lands in and if we can get it into release/5.8 - cc @beccadax or @DougGregor would know, but I guess it depends on quickness of the SE review

/// 1. The ``continuation`` which should be retained by the producer and is used
/// to yield new elements to the stream and finish it.
/// 2. The ``stream`` which is the actual ``AsyncStream`` and
/// should be passed to the consumer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting seems off, 2 spaces

@FranzBusch FranzBusch force-pushed the fb-async-stream-factories branch from 4326a34 to 1186392 Compare January 11, 2023 13:12
@ktoso ktoso added swift evolution implemented Flag → feature: A feature that was approved through the Swift evolution process and implemented concurrency Feature: umbrella label for concurrency language features labels Jan 11, 2023
@FranzBusch FranzBusch force-pushed the fb-async-stream-factories branch from 1186392 to e82417b Compare January 11, 2023 13:13
@FranzBusch
Copy link
Member Author

@swift-ci please smoke test

@FranzBusch FranzBusch force-pushed the fb-async-stream-factories branch from e82417b to bc0053d Compare January 11, 2023 16:40
@FranzBusch
Copy link
Member Author

@swift-ci please smoke test

@beccadax beccadax requested a review from phausler January 11, 2023 19:10
@beccadax
Copy link
Contributor

@swift-ci build toolchain

@ktoso
Copy link
Contributor

ktoso commented Mar 3, 2023

Seems the review is converging on using a tuple, might want to update this PR @FranzBusch :)

@FranzBusch FranzBusch force-pushed the fb-async-stream-factories branch from bc0053d to a5fb888 Compare March 3, 2023 10:00
@FranzBusch
Copy link
Member Author

@swift-ci please smoke test

@FranzBusch FranzBusch force-pushed the fb-async-stream-factories branch from a5fb888 to 26a089e Compare March 3, 2023 14:01
@ktoso
Copy link
Contributor

ktoso commented Mar 4, 2023

Since a commit was pushed after the last trigger:

@swift-ci please smoke test

@ktoso
Copy link
Contributor

ktoso commented Mar 4, 2023

ABI test failure, missing annotations on the following:


+Func AsyncStream.makeStream(of:bufferingPolicy:) is a new API without @available attribute
+Func AsyncThrowingStream.makeStream(of:throwing:bufferingPolicy:) is a new API without @available attribute

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested how to fix the ABI checker issue inline; I believe that's the right combination but @tshortli could double check that.

@ktoso ktoso requested a review from tshortli March 4, 2023 02:53
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
@FranzBusch
Copy link
Member Author

@swift-ci please smoke test

1 similar comment
@FranzBusch
Copy link
Member Author

@swift-ci please smoke test

@airspeedswift airspeedswift merged commit 791b2a0 into swiftlang:main Mar 5, 2023
@FranzBusch FranzBusch deleted the fb-async-stream-factories branch March 6, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features swift evolution implemented Flag → feature: A feature that was approved through the Swift evolution process and implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants