Skip to content

Implement Async File Methods#481

Merged
fgreinacher merged 9 commits intomasterfrom
unknown repository
Jun 7, 2019
Merged

Implement Async File Methods#481
fgreinacher merged 9 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 3, 2019

Please give any feedback - this may just be a starting point for implementing async methods.

Mocks are just passthroughs to sync methods.

Async methods are not available in .NET4 or .NET Standard, so added compilation contexts for .NET Core 2.0

Mocks are just passthroughs to sync methods.

Async methods are not available in .NET4 or .NET Standard, so added compilation contexts for .NET Core 2.0
Comment thread System.IO.Abstractions/IFile.cs Outdated
Comment thread System.IO.Abstractions/System.IO.Abstractions.csproj Outdated
@fgreinacher
Copy link
Copy Markdown
Contributor

Thanks for working on this! Glad to merge as soon as CI is happy!

Copy link
Copy Markdown
Contributor

@robkeim robkeim left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @Fedoranimus. I've added a couple of comments regarding your implementation. Let me know if you have any questions and I can explain in more detail.

Comment thread System.IO.Abstractions.TestingHelpers/MockFile.cs Outdated
Comment thread System.IO.Abstractions.TestingHelpers/MockFile.cs Outdated
@304NotModified
Copy link
Copy Markdown
Contributor

This fixes #441, isn't?

@robkeim
Copy link
Copy Markdown
Contributor

robkeim commented Jun 3, 2019

@304NotModified correct

@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 4, 2019

Note: This PR doesn't have all of the wrappers implemented - working through that still.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 4, 2019

I've implemented all the FileWrappers. Please review and let me know if you have any additional feedback. Thanks!

Comment thread System.IO.Abstractions/FileWrapper.cs Outdated
@robkeim
Copy link
Copy Markdown
Contributor

robkeim commented Jun 5, 2019

@Fedoranimus looks like you missed removing the async keyword in one file... I went ahead and added a comment.

Comment thread System.IO.Abstractions/FileWrapper.cs Outdated
Copy link
Copy Markdown
Contributor

@robkeim robkeim left a comment

Choose a reason for hiding this comment

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

Changes look great thanks for doing this @Fedoranimus

@fgreinacher
Copy link
Copy Markdown
Contributor

Great, thanks also from my side! One last thing: Please update the base version in https://github.com/System-IO-Abstractions/System.IO.Abstractions/blob/master/version.json to 6.0 as we try to follow SemVer and this is a breaking change for implementers.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 6, 2019

@fgreinacher Done.

@fgreinacher fgreinacher merged commit fb13e57 into TestableIO:master Jun 7, 2019
@christianrondeau
Copy link
Copy Markdown
Contributor

I noticed ReadAllLinesAsync is not available with .NET Core 3 preview 7, looks like the precompiler condition trips up here. I wanted to ask here before opening an issue, maybe there's something I missed?

@fgreinacher
Copy link
Copy Markdown
Contributor

Is this a new API of .NET Core 3? If yes, thats fine because we don’t target .NET Core 3 at the moment. If no, it’s a bug. In both cases feel free to open an issue for that. Thanks!

@christianrondeau
Copy link
Copy Markdown
Contributor

No it's not, I believe the problem is that there's a precompiler #if specifically for netstandard 2.0

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