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

Refactor deploy_plugins() in deploy_plugins.py #328

Open
arm4b opened this issue Oct 20, 2022 · 0 comments
Open

Refactor deploy_plugins() in deploy_plugins.py #328

arm4b opened this issue Oct 20, 2022 · 0 comments
Labels
enhancement ✨ New feature or request good first issue Good for newcomers refactor

Comments

@arm4b
Copy link
Member

arm4b commented Oct 20, 2022

This is an unfinished refactoring sub-task coming from the Add Pylint as a python static code analyzer #298.

The deploy_plugins() function in deploy_plugins.py is failing pylint check on:

scripts/plugins/deploy_plugins.py:24:0: R0914: Too many local variables (36/15) (too-many-locals)
scripts/plugins/deploy_plugins.py:24:0: R0912: Too many branches (19/12) (too-many-branches)
scripts/plugins/deploy_plugins.py:24:0: R0915: Too many statements (104/50) (too-many-statements)

The function has more than 250+ lines and pylint with multiple warnings is a great indicator that it needs to be restructured and divided into smaller logical pieces. For instance:

  • Extract it into a new Plugin class perhaps with another layer PluginDeployer (note: PluginInstaller Refactor install_plugins() in install_plugins.py #329 ) with multiple methods, so the interface would be Plugin.deploy() that we can call while iterating over each plugin in the sequence.
  • Then another class PluginConfig would handle plugin config parsing and processing.

The check is silenced for now, don't forget to unsilence it when the refactoring is done.

@arm4b arm4b added enhancement ✨ New feature or request good first issue Good for newcomers refactor labels Oct 20, 2022
arm4b added a commit that referenced this issue Oct 20, 2022
Silence for now, refactor this function in the future.
> R0914: Too many local variables (36/15) (too-many-locals)
> R0912: Too many branches (19/12) (too-many-branches)
> R0915: Too many statements (104/50) (too-many-statements)

See: #328
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request good first issue Good for newcomers refactor
Projects
None yet
Development

No branches or pull requests

1 participant