-
Notifications
You must be signed in to change notification settings - Fork 256
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
refactor: improve and consolidate interfaces #906
Conversation
- Enable Nullable reference types - Consolidate naming in factory methods - Add missing methods
@fgreinacher : |
tests/TestableIO.System.IO.Abstractions.Wrappers.Tests/ApiParityTests.cs
Outdated
Show resolved
Hide resolved
@fgreinacher :
|
Sorry @vbreuss, I'll have a look today! |
src/TestableIO.System.IO.Abstractions.Wrappers/FileSystemWatcherBase.cs
Outdated
Show resolved
Hide resolved
src/TestableIO.System.IO.Abstractions/IFileSystemExtensionPoint.cs
Outdated
Show resolved
Hide resolved
src/TestableIO.System.IO.Abstractions/IFileSystemExtensibility.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @vbreuss, this looks great, I left some suggestions/questions!
…erBase.cs Co-authored-by: Florian Greinacher <[email protected]>
Thanks for the review, @fgreinacher ! I implemented all review findings and answered your questions :-) Are the next planned steps also OK for you?
|
Co-authored-by: Florian Greinacher <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for your hard work here!
The next steps are also fine from my side 👍
@fgreinacher : Is there anything I can do? |
All good, sometimes it just takes a bit 🐌 |
- Apply renamings done in System.IO.Abstractions (see [Pull Request #906](TestableIO/System.IO.Abstractions#906)) - Use `global using Testably.Abstractions.FileSystem` so that it can be more easily replaced with another namespace - Add `IFileSystemEntity` to `IFileSystemInfo` - Remove nesting from IWaitForChangedResult
Make the following changes for the interfaces:
New
; so instead ofnew DirectoryInfo(path)
you should useIFileSystem.DirectoryInfo.New(path)
. The previous existing factory methods were kept and marked asObsolete
.Wrap
method to the factory interfaces that can be used instead of a direct cast (which is not possible for interfaces) to create a wrapped interface from a "System.IO" type.WriteAllLinesAsync
with array overload, as it is not defined inSystem.IO
and overload resolution will automatically use theIEnumerable
overload.ResolveLinkTarget
andCreateAsSymbolicLink
FileSystemStream
.Note: This should fix the issues from feat: Add
Name
toIFile.Create
(and others) #793 , right?Added the following extensibility measures:
IFileSystem FileSystem
property to more classes to enable implementing extension methods.This was simplified by adding a base interface
IFileSystemExtensionPoint
IFileSystemExtensibility Extensibility
property onIFileSystemInfo
andIFileSystemStream
which grants access to the underlying wrapped instance or to a metadata dictionary to store additional information together with the instance.