-
Notifications
You must be signed in to change notification settings - Fork 106
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
ENHANCEMENT: option to hide global files #177
Conversation
I'm not entirely sure how closely this module wants to follow semver, but this is a new feature and so shouldn't make it into the 1.0 branch. Instead it should be against |
@dhensby thanks for reviewing this. Given that this is a non-breaking enhancement I believe it should be in the |
For SemVer a change of this kind has to go into the minor release (1.x) not a patch (1.0.x) and therefore this needs to be against master (which I assume is the 1.x release). I can add a PR to alias master as 1.x so that you can do "~1.0" as a version constraint and get the master branch if your tracking dev dependencies. |
Hmm, it does seem that master is tracking ss 3.2 and thus actually a 2.0.x release. @tractorcow thoughts? |
I don't think we need to follow semantic versioning too closely, outside of the framework / cms (and their dependencies), although it's still a good model to follow. :) My question is, how are global assets managed if they are permanently filtered out? Are admins not even allowed to view them, or are they just unrecoverable? |
@tractorcow the idea is that global files would be accessible from the main site, while site-specific files would be accessible from each subsite. |
* @config | ||
* @var bool | ||
*/ | ||
private static $show_global_folders = true; |
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.
This config can't be turned off via YML, due to merge rules ignoring falsey values being merged with truthy.
Could we flip the meaning so that the default is false? E.g. 'hide_global_folders = false'.
We are now following semver on this module, so um, can you please re-open this PR on master? Sorry to be a bummer! |
@tractorcow okay, I haven't worked on the affected project for a while. I'll revisit this, once I'll work on upgrading that project. |
Allowing to hide global folders in
AssetAdmin
.Config example: