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

Basics: remove AsyncFileSystem as unused #6436

Merged
merged 1 commit into from Apr 15, 2023

Conversation

MaxDesiatov
Copy link
Member

@MaxDesiatov MaxDesiatov commented Apr 14, 2023

Motivation:

Since FileSystem is now Sendable, we no longer need to keep AsyncFileSystem, which served only as a workaround for non-sendability. It wasn't non-blocking under the hood anyway, and a blocking FS I/O API would have to be designed from scratch anyway, not as a wrapper for the existing blocking APIs.

Modifications:

AsyncFileSystem actor declaration and its uses were removed. FileSystem can be used directly in our async code instead.

Result:

Reduced the amount of redundant code, FS code is now easier to understand when async/await are used.

Since `FileSystem` is now `Sendable`, we no longer need to keep `AsyncFileSystem`, which served only as a workaround for non-sendability. It wasn't non-blocking under the hood anyway, and a blocking FS I/O API would have to be designed from scratch anyway, not as a wrapper for the existing blocking APIs.
@MaxDesiatov
Copy link
Member Author

@swift-ci smoke test

@shahmishal
Copy link
Member

@swift-ci smoke test macOS self hosted

1 similar comment
@shahmishal
Copy link
Member

@swift-ci smoke test macOS self hosted

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) April 14, 2023 17:43
@yim-lee
Copy link
Member

yim-lee commented Apr 15, 2023

@swift-ci please test Windows platform

@MaxDesiatov MaxDesiatov merged commit 717aec7 into main Apr 15, 2023
5 checks passed
@MaxDesiatov MaxDesiatov deleted the maxd/remove-asyncfilesystem branch April 28, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants