-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Refactor core into plugins with command and subcommand scripts #592
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
|
For the record, the CI failure is due to #568. |
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. "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. 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. This commit also removes the nginx-vhosts plugin's functionally-empty "commands" hook script, for clarity and cleanliness.
|
👍 |
|
For the record, in the two months since I proposed this PR (or the four months since it was put together for #458), I've decided to create a new project (https://github.com/plushu/plushu) rather than continuing to try to push a complete overhaul of everything through Dokku. One thing I've noticed, as I've been referring back to this branch's code as reference for Dokku, is that, if this were to be merged, the |
|
@josegonzalez for whatever reason this seems cleaner I like the idea of moving commands into plugins but I'm not completely sold on the premise to restructure subcommands. Thoughts? |
|
For what it's worth, this subcommand structure is how Plushu has been working since the beginning, and it's worked just fine over the course of six months and ~50 plugins (in which it has reached and surpassed feature parity with Dokku). |
|
@stuartpb the intention of my comment was not meant to judge the functionality of this structure. Instead, I'd like to understand the reasoning behind it. dokku currently has all commands tucked away in a single commands file per plugin. To me that seems, very neat, concise, and makes it dead simple to execute against. i.e. Rather than Since I lack the understanding of the additional benefits, its seems to just be personal preference and the former is easier to understand for me than the latter. Can you clarify why the subcommand pattern (i.e. |
|
The big win that I've seen in Plushu is that there's way less non-deterministic boilerplate in each plugin. On top of making it way easier to spin up new plugins, when you don't have every plugin sticking its thumbs in the command parser, there's way less opportunity for a bug in a one-off plugin to break every other plugin in the system. |
|
Can you give an example? |
|
An example of one plugin needlessly breaking command handling for every other plugin? Say |
|
Or, say that its name starts with |
|
Ah that completely makes sense and this implementation bypasses that because you are executing the plugin command directly instead of iterating and executing every plugin's commands file every time you attempt to run any command. Got it. Thanks! |
|
@michaelshobbs thoughts on this PR, or ideas we can/should pull from it? |
|
Yeah I think we can/should use this pattern. I can take a swing at it. Let's leave this open for now. |
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.
Miscellany
This commit also removes the nginx-vhosts plugin's functionally-empty "commands" hook script, for clarity and cleanliness.
(This is effectively a resubmission of #458, as it was remade after taking #312 into account, squashed and rebased onto the current master.)