silverstripe / vendor-plugin-helper

BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

TODO Retire this repo #6

Open tractorcow opened 6 years ago

tractorcow commented 6 years ago

I think we can retire this module now. It was what we used prior to composer vendor-expose.

I think that leaving this open left some unintentional ambiguity about if it was necessary to maintain. Sorry about that!

tractorcow commented 6 years ago

cc @chillu @maxime-rainville

maxime-rainville commented 6 years ago

I had the same idea https://github.com/silverstripe/vendor-plugin-helper/issues/4#issuecomment-434913133

Unfortunately, if you run composer vendor-expose composer will still try to call the activate method on other composer plugin. I've confirmed this by sticking a die statement in SilverStripe\RecipePlugin\RecipePlugin::activate().

tractorcow commented 6 years ago

Oh I see, you don't want other plugins to allow custom code to be executed right?

tractorcow commented 6 years ago

I would suggest that we recommend the general public use vendor-expose command, but we separate this into an independent internal tool (albeit open sourced) if you think it's still necessary to maintain for platform.

maxime-rainville commented 6 years ago

That shouldn't be a problem. It's not something we actively advertise anywhere. People really need to go out of their way to find it.

We can maybe add a line to the read me along the line of "This is an internal tool ... you really should be using vendor-expose instead". Or maybe get the repo transferred under Silverstripeltd.

chillu commented 6 years ago

How much maintenance is there for this tool, separate from vendor-expose? Happy to keep it open source but set the maintenance expectations in the read me as Max mentioned. If you move this repo, please check with SilverStripe operations. I think the original driver was to run composer install --no-scripts to avoid arbitrary code execution on platform environments. Some of this has now been isolated via AWS CodeBuild (I think), and in general its best practice to spin up sandboxes for build steps. But we might still have this requirement on other platforms.

maxime-rainville commented 6 years ago

5 tries to shift more of the logic to use vendor-plugin. Most of the change tries to maintain backward compatibility so people can transparently upgrade without having to rewrite their scripts.

If we drop the requirements for backward compatibility, this module will not be a lot of maintenance.