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

Named params #1091

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Named params #1091

wants to merge 4 commits into from

Conversation

IceBreeze
Copy link
Contributor

This PR introduces named parameters for the plugins while maintaining backwards compatibility.
It should also allow for conversion of existing parameters without losing the configuration.

To help you in the review, basically get_plugin_parameters() now return an hash instead of an array.
It can be feed directly to the new plugins entry point, but, for the old plugins, it contains the key customargs with the array of values from the DB.
The choice of what to use is made inside the exec_*_plugin subs.

Some issues I want to point out:

  • the hashes $lrr_info and $params can be merged, but this requires adding checks to avoid collisions between the names of internal keys and those chosen by the plugin contributors. Alternatively you could change the names of the "internal" keys (eg: archive_id to _archive_id), but this will break the compatibility.
  • oneshot_param is now duplicated in $params as $params->{'oneshot'} because I think it should be moved there if we keeps the two hashes separated.
  • login plugins load, but I don't have an account to fully test them

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.

1 participant