I've been following the changes to IFileSystem closely since any changes directly affect my own project and any other implementations out in the wild.
There's a few issues that are bothering me:
A new property has been added to the API that is an implementation detail not an API design one. CanAddPhysicalhttps://github.com/umbraco/Umbraco-CMS/blob/d4edbc8e791a9d944c737e0882b973672787147a/src/Umbraco.Core/IO/IFileSystem.cs#L160
There are multiple issues with this: Not only is it poorly named (Yes I can add a physical file, it's just going to require a HttpRequest), but more importantly, it is not a concern of the API itself, violating the Interface Segregation Principle. If there is a design flaw in the downstream API's that require this problem then I suggest fixing it there.
There are no async methods which is a major performance killer on networked implementations like Azure or AWS, or even a network drive. See https://github.com/umbraco/Umbraco-CMS/issues/3323 that highlights this issue but gets closed because a performant filesystem isn't seen as a high enough priority.
I'm really concerned about the state of these API's and strongly suggest that Umbraco HQ work with the community to design the APIs properly before launching V8.
and strongly suggest that Umbraco HQ work with the community to design the APIs properly
Could be we are concerned with these APIs too. Could be we totally agree with your remarks, and are painfully aware of all the many principles and patterns the code is violating.
Could be we do work with the community to design APIs. The code is open, treat it as an RFC. Which is precisely what you have done, sharing your concerns. Excellent. It works :-)
Could be there are so many areas that need improvement, step by step, and only a finite amount of time, that we simply cannot address these concerns now. Either someone has time for a PR, or it has to wait.
What bothers me is that we seem to lack a nice way to keep track of this feedback, other than an issue that gets closed (and lost?). Maybe we need a special kind of issue/task or whatever.
IFileSystem design implementation
I've been following the changes to
IFileSystem
closely since any changes directly affect my own project and any other implementations out in the wild.There's a few issues that are bothering me:
CanAddPhysical
https://github.com/umbraco/Umbraco-CMS/blob/d4edbc8e791a9d944c737e0882b973672787147a/src/Umbraco.Core/IO/IFileSystem.cs#L160 There are multiple issues with this: Not only is it poorly named (Yes I can add a physical file, it's just going to require a HttpRequest), but more importantly, it is not a concern of the API itself, violating the Interface Segregation Principle. If there is a design flaw in the downstream API's that require this problem then I suggest fixing it there.I'm really concerned about the state of these API's and strongly suggest that Umbraco HQ work with the community to design the APIs properly before launching V8.
Could be we are concerned with these APIs too. Could be we totally agree with your remarks, and are painfully aware of all the many principles and patterns the code is violating.
Could be we do work with the community to design APIs. The code is open, treat it as an RFC. Which is precisely what you have done, sharing your concerns. Excellent. It works :-)
Could be there are so many areas that need improvement, step by step, and only a finite amount of time, that we simply cannot address these concerns now. Either someone has time for a PR, or it has to wait.
What bothers me is that we seem to lack a nice way to keep track of this feedback, other than an issue that gets closed (and lost?). Maybe we need a special kind of issue/task or whatever.
is working on a reply...