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

Support simple Composer-based Command plugin mechanism #604

Closed
wants to merge 5 commits into from

Conversation

greg-1-anderson
Copy link
Member

based on the PSR-4 autoload namespace.

Overview

This pull request:

  • Fixes a bug
  • Adds a feature
  • Breaks backwards compatibility
  • Has tests that cover changes

Summary

Simple command plugin mechanism.

@greg-1-anderson
Copy link
Member Author

This implements a plugin scheme based on the idea of @thom8 in acquia/blt#1793.

The examples RoboFile has been repackaged as an example Robo plugin.

@greg-1-anderson
Copy link
Member Author

The command plugin commandfile collection code should be factored out of the runner; beyond that, though, this is pretty simple and works well.

@greg-1-anderson
Copy link
Member Author

Another alternative would be to use something like Flex, but that would be a lot more heavyweight.

We could also consider using a "files" autoload entry in each plugin, and have each plugin register itself at autoload time. This would require declaring a global function with a static or global variable, or use a singleton to cache the registered classes. The implementation in this PR seems better to me than that, though.

* 2. Add command files to the specified directory. The command files must
* have names ending `Commands.php`. They must either be immediately
* inside the directory specified in the psr-4 autoload section, or they
* may be anywhere inside a directory named `Commands`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about hooks?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can put hooks in any command file. If you prefer, we could also define a default naming convention for files that are intended to have only hooks in them.

Note, however, that there is no difference in behavior between command files and hook files; you could put commands in hook files, or visa-versa.

@grasmash
Copy link
Contributor

Testing here: https://github.com/acquia/blt/pull/1821/files

src/Runner.php Outdated
}

$discovery = new CommandFileDiscovery();
$discovery->setSearchLocations(['Commands']);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grasmash Would it make things better for your use cases if the search locations could be altered via a method call that specified the locations? Alternately, we could factor this out into a separate class, and you could call it explicitly with the CommandFileDiscovery object of your choice.


protected function getPluginCommandClasses()
{
$commandClasses = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind making this method public? That way, an application that leverages robo can call it and build its own command classes. You may also want to consider making getRoboFileCommands() public as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no. I don't think it would be good for folks to be calling methods of the runner to do this, but we could provide a plugin discovery class that provided this service. Then Runner could use the new class.

@thomscode
Copy link
Contributor

I'm really looking forward to this feature. Is there any further momentum that can be made with this? Looking at the TravisCI report the issue was with a PHP 7.0.7 vs 7.0.8 issue with Symfony 3.3.8.

@greg-1-anderson
Copy link
Member Author

See alternate implementation in #671

@greg-1-anderson greg-1-anderson deleted the plugin-commands branch October 17, 2019 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants