silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
720 stars 820 forks source link

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

Open GuySartorelli opened 1 month ago

GuySartorelli commented 1 month ago

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

maxime-rainville commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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