feat: plugin system hooks updated

[joshLong145]

implementation of proposal #2913

Addition of hooks on plugins
Allows for pre, post and clean up operations on pre existing ignite commands.

updated with develop based on pr #3011

[aljo242]

Thanks so much for the great work ! Here is some feedback so far.

Also please make sure to run make lint and make format!

[aljo242]

Looks like we also need a changelog entry (example).

[tbruyelle]

@joshLong145 could you enable Allow edits from maintainers. ? I would like to add a commit that reinforce the test TestLinkPluginHooks. It's currently too weak since it only checks that PreRun or PostRun are not nil. Thanks

This is a checkbox in your PR.

[joshLong145]

@joshLong145 could you enable Allow edits from maintainers. ? I would like to add a commit that reinforce the test TestLinkPluginHooks. It's currently too weak since it only checks that PreRun or PostRun are not nil. Thanks

This is a checkbox in your PR.

I think i have enabled it. if there are specifics you want to see in the tests I can add it.

[tbruyelle]

@joshLong145 could you enable Allow edits from maintainers. ? I would like to add a commit that reinforce the test TestLinkPluginHooks. It's currently too weak since it only checks that PreRun or PostRun are not nil. Thanks
This is a checkbox in your PR.

I think i have enabled it. if there are specifics you want to see in the tests I can add it.

Thanks but it's not completely clear in my head, I have to draft it by coding!

[tbruyelle]

@joshLong145 I pushed a change to reinforce TestLinkPluginHooks. The test now executes all the created commands and ensures that any hook attached to one of the commands is also executed.

For that I had to change the signature of the plugin.Interface, but that was intended since if I remember well I already told you that ExecuteHook* should take the whole Hook as a parameter and not only the name.

If you look at the cases, you may find weird scenarios when the same hook is attached multiple times to the same command. At the current stage, this works, but we can choose to disable.

I will provide a second commit to update the go.mod.plush with the latest hash.

[joshLong145]

@joshLong145 I pushed a change to reinforce TestLinkPluginHooks. The test now executes all the created commands and ensures that any hook attached to one of the commands is also executed.

For that I had to change the signature of the plugin.Interface, but that was intended since if I remember well I already told you that ExecuteHook* should take the whole Hook as a parameter and not only the name.

If you look at the cases, you may find weird scenarios when the same hook is attached multiple times to the same command. At the current stage, this works, but we can choose to disable.

I will provide a second commit to update the go.mod.plush with the latest hash.

Sounds good. I have some comments to address from @aljo242 i can update the hash as the final commit once those are completed

[tbruyelle]

added flag tests to the existing test suite in ignite/cmd/plugin and ran formatters. having some issues updating the go.mod.plush with the new commit hash @tbruyelle if you could add that it would much appreciated slightly_smiling_face

I don't think it's needed to change the commit hash since your last commits didn't change the plugin interface.

About your change, what you are asserting are the command's arguments, not the command's flags. Can you rename flags to args ?

Passing command's flags to plugin Execute* methods isn't currently supported, it's WIP.

[joshLong145]

added flag tests to the existing test suite in ignite/cmd/plugin and ran formatters. having some issues updating the go.mod.plush with the new commit hash @tbruyelle if you could add that it would much appreciated slightly_smiling_face

I don't think it's needed to change the commit hash since your last commits didn't change the plugin interface.

About your change, what you are asserting are the command's arguments, not the command's flags. Can you rename flags to args ?

Passing command's flags to plugin Execute* methods isn't currently supported, it's WIP.

changed the property to args also tested hooks with passing flags to commands and they pass correctly to the command implementation and hooks.

[tbruyelle]

Tested, works like a charm, thank you @joshLong145, well done!

Note that flags support is ready, but I would like to merge yours first and adapt mine afterward.
#3060

[joshLong145]

Tested, works like a charm, thank you @joshLong145, well done!

Note that flags support is ready, but I would like to merge yours first and adapt mine afterward. #3060

Yes agreed, the implementation for passing flags can be added to hooks as well. Excited to see what is built with this

[tbruyelle]

Yes agreed, the implementation for passing flags can be added to hooks as well. Excited to see what is built with this

Yes I'll add flags support to hooks too.