Skip to content

lastpass lookup: use config manager, improve documentation#5022

Merged
felixfontein merged 2 commits intoansible-collections:mainfrom
lungj:fix-lastpass-lookup
Aug 1, 2022
Merged

lastpass lookup: use config manager, improve documentation#5022
felixfontein merged 2 commits intoansible-collections:mainfrom
lungj:fix-lastpass-lookup

Conversation

@lungj
Copy link
Contributor

@lungj lungj commented Jul 28, 2022

SUMMARY

Cleaned up LastPass lookup plug-in to meet coding standards.

Since the issues in #5012 are a result of me previously basing code off of this, by making this adhere to current standards for this repository, hopefully this will result in less work for reviewers in the future.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lastpass

ADDITIONAL INFORMATION

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug lookup lookup plugin new_contributor Help guide this first time contributor plugins plugin (any type) labels Jul 28, 2022
@github-actions
Copy link

github-actions bot commented Jul 28, 2022 •
edited
Loading

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

felixfontein
felixfontein reviewed Jul 28, 2022
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. Could you please add a changelog fragment? Thanks.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Jul 28, 2022
@felixfontein felixfontein changed the title Cleanup of LastPass lookup plug-in to meet coding standards lastpass lookup: use config manager, improve documentation Jul 28, 2022
@felixfontein
Copy link
Collaborator

felixfontein commented Jul 28, 2022

What's missing are similar changes to the unit tests as in #5012 (comment).

@lungj lungj force-pushed the fix-lastpass-lookup branch from 3cdd0a5 to 8da082f Compare July 28, 2022 19:54
@ansibullbot ansibullbot added tests tests unit tests/unit labels Jul 28, 2022
@lungj lungj force-pushed the fix-lastpass-lookup branch from 8da082f to fed0520 Compare July 28, 2022 20:00
@felixfontein
LastPass lookup: use config manager, improve documentation
93dbdb2 
Co-authored-by: Felix Fontein <felix@fontein.de>
@lungj lungj force-pushed the fix-lastpass-lookup branch from fed0520 to 93dbdb2 Compare July 28, 2022 20:18
felixfontein
felixfontein reviewed Jul 28, 2022
View reviewed changes
@lungj @felixfontein
Update changelogs/fragments/5022-lastpass-lookup-cleanup.yml
42eb64d 
Co-authored-by: Felix Fontein <felix@fontein.de>
felixfontein
felixfontein approved these changes Jul 29, 2022
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. If nobody complains, I'll merge this for the next release.

@lungj
Copy link
Contributor Author

lungj commented Jul 29, 2022 •
edited
Loading

Looks good to me. If nobody complains, I'll merge this for the next release.

I left this as a draft in case there are any new issues identified in #5012 that are also in this plug-in. Maybe we can hold off until that’s also good?

@felixfontein
Copy link
Collaborator

felixfontein commented Jul 31, 2022

I'd prefer to merge this for 5.4.0 no matter whether #5012 is merged by then or not. The most important change here has been made (use of set_options() / get_option()).

@lungj lungj marked this pull request as ready for review July 31, 2022 21:16
@ansibullbot ansibullbot removed the WIP Work in progress label Jul 31, 2022
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Aug 1, 2022
@felixfontein felixfontein merged commit e8e6b9b into ansible-collections:main Aug 1, 2022
@patchback
Copy link

patchback bot commented Aug 1, 2022 •
edited
Loading

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/e8e6b9bbd7b7f970638a453b7b0b81af13885f42/pr-5022

Backported as #5047

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Aug 1, 2022
@lungj @felixfontein @patchback
lastpass lookup: use config manager, improve documentation (#5022)
e4ee2cd 
* LastPass lookup: use config manager, improve documentation

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/5022-lastpass-lookup-cleanup.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: jonathan lung <lungj@heresjono.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit e8e6b9b)
@patchback patchback bot mentioned this pull request Aug 1, 2022
Merged
@felixfontein
Copy link
Collaborator

felixfontein commented Aug 1, 2022

@lungj thanks for fixing this!

@anaconda-renovate-bot anaconda-renovate-bot mentioned this pull request Nov 14, 2022
Merged
1 task
This was referenced Nov 15, 2022
Closed
Merged
Closed
Merged
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Open
Closed
Closed
Closed
Closed
Closed
This was referenced Dec 1, 2022
Open
Closed
@renovate renovate bot mentioned this pull request Dec 14, 2022
Closed
1 task
@renovate renovate bot mentioned this pull request Jan 1, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixfontein felixfontein felixfontein approved these changes

Assignees

No one assigned

Labels

bug This issue/PR relates to a bug lookup lookup plugin new_contributor Help guide this first time contributor plugins plugin (any type) tests tests unit tests/unit

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lungj @felixfontein @ansibullbot