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

Don't require _config/ dir or _config.php file for modules #11254

Closed
GuySartorelli opened this issue May 20, 2024 · 4 comments
Closed

Don't require _config/ dir or _config.php file for modules #11254

GuySartorelli opened this issue May 20, 2024 · 4 comments
Assignees
Milestone

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented May 20, 2024

Currently the ManifestFileFinder only looks for files that are inside a directory with either a _config/ directory or _config.php file, because that's how it detects if something is "a module".

https://docs.silverstripe.org/en/5/getting_started/directory_structure/#module-structure also says

They need to have a _config.php file or a _config/ directory present

We should allow for directories with a composer.json file where the type is set to either silverstripe-vendormodule or silverstripe-theme to also count, since that's the type we say is necessary for modules and themes and some may not have a need for config.

Acceptance criteria

  • modules and themes don't need to have a _config/ directory or _config.php file to be detected as modules, including full manifest detection
  • the _config/ directory and _config.php file detection is left in place as a fallback

PRs

@maxime-rainville
Copy link
Contributor

Any reason why we would wait for CMS 6 to do this change. This seems like something that could happen in CMS 5.3 without any downside.

@GuySartorelli
Copy link
Member Author

Any reason why we would wait for CMS 6 to do this change

Who knows if someone has a module that has the silverstripe-vendormodule type but they don't want to be considered a module, so their hacky way to do that was to not have the dir/file needed to pick it up?

Seems unlikely, but I'd take a risk-averse approach. There's very little risk for sure, but I'd argue that in this case the benefit of doing it in CMS 5 is so low that there's no harm in doing it for 6 and just not upsetting the few people that might be upset by it.

@GuySartorelli
Copy link
Member Author

GuySartorelli commented May 28, 2024

That said, I don't feel overly strongly about it. Like I say there's both little risk and little reward.

@GuySartorelli
Copy link
Member Author

PRs merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants