-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
passwordstore lookup: allow to pass options as lookup options #5444
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
CC @baierjan @bergmannf @grembo @phaer @TheLastProject since you contributed to this plugin before. Would be glad if you could review this / try this out! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is quite a big change, but it definitely looks cleaner that way. Fine by me. 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is generally an improvement and improves consistency with other modules. But it will hurt people a bit as they'll have to refactor their Ansible code. Definitely needs a major release and needs to be marked as a major change imho.
Also note I haven't tested this, just looking at how the code looks to me.
But why? Everything that worked so far still works. (Except if you passed lookup parameters and expected them to not work.) This PR only allows a new (better) way to provide options. The old way still works (it should get deprecated eventually, but definitely not now). |
Nevermind them, misunderstood the code in that case :) |
@baierjan @TheLastProject thanks a lot for reviewing this! |
…e-collections#5444) * Allow to pass options as lookup options. * Adjust tests.
…e-collections#5444) * Allow to pass options as lookup options. * Adjust tests.
…e-collections#5444) * Allow to pass options as lookup options. * Adjust tests.
…e-collections#5444) * Allow to pass options as lookup options. * Adjust tests.
SUMMARY
Something which should have been done a long time ago: allow to pass in options as lookup options instead of insisting that they have to be parsed from the terms.
ISSUE TYPE
COMPONENT NAME
passwordstore lookup plugin