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

Why are we monkey patching TaskExecutor instead of providing a proper VarsModule? #8

Open
lpirl opened this issue Nov 11, 2019 · 2 comments

Comments

@lpirl
Copy link
Contributor

lpirl commented Nov 11, 2019

Is there anything (apart from checking become) why monkey patching TaskExecutor is a superior solution to just writing a VarsModule?

IMHO the present approach torpedoes the idea of "vars plugins" (and the Law of Demeter).

In order to gain readability and comprehensibility (which is surely is subjective) for investigating #4, a refactored version of your wonderful plugin resulted. In this refactored version, you can find an example implementation as a "vars plugin".

The refactored version also contains the changes I pull-requested (plus: it moves all functions out the scope of the module, fixes to be more compliant with PEP 8, etc.).

@Nekmo
Copy link
Owner

Nekmo commented Apr 21, 2020

Thanks for your work @lpirl, I am going to test your solution using your var_module. I don't remember the reason to use monkey patching but if possible using a vars_module is always preferable. Thanks!

I also apologize for the delay in answering.

@lpirl
Copy link
Contributor Author

lpirl commented Apr 21, 2020

I also apologize for the delay in answering.

Take your time – we're all doing this voluntarily. :)

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

No branches or pull requests

2 participants