-
-
Notifications
You must be signed in to change notification settings - Fork 2k
refactor plugin structure. closes #592 #1039
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
Conversation
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.
I didn't want to rename nginx-vhosts to nginx. Seemed ambiguous....
|
Do we really want to move |
|
Currently I don't have an answer for the 'is not a dokku command' scenario. Any suggestions are welcomed. EDIT: scratch that. found a bug instead. 😄 |
75fee7b to
ac7ed79
Compare
|
Note: We'll need to merge the other pending plugin changes into the is branch before merging this into master. |
Conflicts: plugins/00_dokku-standard/commands plugins/config/commands
faffeae to
ce29b22
Compare
Conflicts: plugins/domains/commands
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.
Does this handle legacy plugins? If so, do we have a test for that?
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.
Yes and yes.
We still for loop here: https://github.com/progrium/dokku/blob/592_mh-plugin-structure-refactor/dokku#L157-L167
The core, git, and plugins plugins still have commands files.
|
Did you add any new pluginhooks? If so, please doc them. Unless I missed something, I think this is quite shippable :) |
|
No new hooks. This was only a |
|
Ship it! |
|
Actually let's hold off. I want to check that switching to this would be compatible with plugn, the new pluginhook @progrium is working on. |
|
The main incompatibility is one of the main changes this introduces. And it's actually more associated with the "new style" Bash approach. Namely that functions and sourcing is used over separate shell scripts. This is actually very different than pluginhook and plugn currently, but plugn will support a mechanism to help source files from a plugin as well as the traditional hook script style. As a simple example (that doesn't reflect how it might specifically work), instead of a command hook and many subcommand hooks, there would be a commands.bash file with each command defined in functions. First issue you might see is that this means it can only be Bash. Yes, well it all is only Bash. But I'm looking at a way to make it try this first and then fall back to shell scripts. Even without fallback design, it would mean the plugin author would have to define wrapper functions to call their shell scripts. Not ideal, but just pointing out it doesn't preclude shell scripts. Arguably this is just another re-organization issue that's not fundamentally different. Normally we could just accept this PR and then change it again with plugn ... except we're changing the Dokku plugin API with both. And if we're going to do that, we should try to do it only once. My suggestion would be to keep these changes around as a branch for reference, since there a good structural ideas we want to move towards. The specifics of what it should be are just different. However, there's other good stuff in here that doesn't change the API, and I think we can merge those in pretty easily. |
|
@progrium what would be the ideal plugin structure? If we're going to reorganize plugins, we may as well do it now - and the commands/subcommands are now separated so it would be maybe be easier to move to whatever structure we choose. |
|
Plugn uses the same directory structure as pluginhook. But my point is more On Thu, Mar 26, 2015 at 9:29 AM, Jose Diaz-Gonzalez <
Jeff Lindsay |
|
@progrium well, given that redoing how we handle plugins was something you wanted to work on - and in order to decrease the amount of work we throw at you! - how do you recommend we proceed if someone else made the pluginhook/plugn changes? It seems as though |
|
@progrium: can you enumerate the ways forward and their drawbacks? what bits don't change the api and should be merged? |
|
@progrium bump :) |
|
Superseded by #1228. |
inspired by #592
from @stuartpb:
This change makes the mapping of plugins to commands in Dokku significantly more consistent, giving each existing Dokku command its own plugin, and each plugin a dedicated location for a provided command and/or subcommands.
Beyond the navigational advantages, this allows individual commands / components of Dokku to be disabled or overridden at the plugin level, without having to muck around patching scripts that provide unrelated commands.
Commands and subcommands
"command" scripts, like a hook, are executed when the name of a plugin is
called: for instance, "dokku build" calls the "build" plugin's "command"
script (plugins/build/command).
"subcommand" scripts are called for subcommands of plugins, as namespaced with
a colon: for example, "dokku backup:export" calls the "export" subcommand
script of the "backup" plugin (plugins/backup/subcommands/export).
Plugins that do not use their name for commands/subcommands (such as the git plugin, or plugins that wrap other commands) can continue to use the "commands" hook, which remains fully supported and backward-compatible.
Updates to documentation
Each plugin / command gets a README describing its purpose and implementation.
Additionally, the "help" text describing its commands' usage is moved out to a separate "help.txt" file, which is then read and (potentially) columnized when running the "help" command.
This commit also removes the "warning" in the plugins directory - Dokku's stability, hooks and all, is
covered by the main Dokku README.