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

Adds support for AsyncStream #4

Merged
merged 5 commits into from
Jun 17, 2024
Merged

Conversation

mcritz
Copy link
Contributor

@mcritz mcritz commented May 28, 2024

This PR:

  • Adds AsyncStream support to FileMonitor for modern Swift Structured Concurrency support
  • Bumps the Package build requirement to 5.9, breaking backward compatibility

Benefits:
Users can change callback based system with for await event in fileMonitor.stream { ... }

Drawbacks:
I have forced the Package to use Swift 5.9 instead of using #available or @available. We could use an availability check to avoid breaking backward compatibility. OR we could release a new, breaking release of this Package using Github releases.

I’m open to either approach, but we should pick one.

@mcritz
Copy link
Contributor Author

mcritz commented Jun 6, 2024

@petershaw Any interest in taking this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in favor of offering a stream to consume events.
What would you think about a method that returns the stream (and sets an optional continuation member) rather than initialing the stream by default?

@tklae
Copy link

tklae commented Jun 14, 2024

Thanks for adding this and sorry for the late reply, somehow we missed it... We will have a look and get back to you, soon!

@KrisSimon
Copy link
Contributor

Hey @mcritz
I am totally fine with a change to the swift 5.9 dependency!

I'll manage to take a look to the implementation at the beginning of next week.
Pretty cool at the first look! Thank you very much for contribution.

@KrisSimon
Copy link
Contributor

On Linux the FileMonitorTests.testLifecycleChange hangs.
@mcritz can you take a look into it?

@KrisSimon
Copy link
Contributor

@mcritz I split test into two and fixed the problem.
Let's wait for the CI confirmation. ;)

@KrisSimon KrisSimon merged commit ef22f14 into aus-der-Technik:main Jun 17, 2024
2 checks passed
@mcritz
Copy link
Contributor Author

mcritz commented Jun 17, 2024

@KrisSimon Thanks for fixing that. Happy to have made a contribution!

@KrisSimon
Copy link
Contributor

@mcritz thank you very much for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants