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

Add ReadMe plugin #106

Merged
merged 15 commits into from
Dec 16, 2022
Merged

Add ReadMe plugin #106

merged 15 commits into from
Dec 16, 2022

Conversation

kanadgupta
Copy link
Contributor

@kanadgupta kanadgupta commented Dec 12, 2022

Hi folks, big 1Password fans over here at ReadMe! Looking forward to being a part of this exciting new ecosystem 💙

I still have a few small outstanding TODOs in this PR (that I'll address before officially marking this as ready for review), but the main outstanding issue is that I'm running into is that the env variable RDME_API_KEY isn't properly being provisioned in my testing 😕 see #107 for more information!

EDIT: fixed, thanks for your help @SimonBarendse @florisvdg!

Closes #108

@kanadgupta kanadgupta marked this pull request as ready for review December 14, 2022 00:45
erunion
erunion previously approved these changes Dec 14, 2022
Copy link

@erunion erunion left a comment

Choose a reason for hiding this comment

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

:nice-rgb:

Copy link
Member

@SimonBarendse SimonBarendse left a comment

Choose a reason for hiding this comment

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

Love it! Thanks for contributing @kanadgupta! I've just left a few last nitpicks for you to consider, overall this looks great! I love the detail in your MR, including using the subdomain as naming suggestion for the 1Password item, the inclusion of the configuration file importer and adding these tests! 🙌

Welcome to the ecosystem ReadMe! 😍

plugins/readme/api_key.go Outdated Show resolved Hide resolved
plugins/readme/api_key.go Outdated Show resolved Hide resolved
@kanadgupta
Copy link
Contributor Author

(oops, didn't mean to remove the review request for @DCKcode, sorry!)

@DCKcode
Copy link
Collaborator

DCKcode commented Dec 15, 2022

This looks great - thanks for your contribution!

DCKcode
DCKcode previously approved these changes Dec 15, 2022
Co-authored-by: Simon Barendse <SimonBarendse@users.noreply.github.com>
SimonBarendse
SimonBarendse previously approved these changes Dec 15, 2022
DCKcode
DCKcode previously approved these changes Dec 15, 2022
volodymyrZotov
volodymyrZotov previously approved these changes Dec 15, 2022
@kanadgupta
Copy link
Contributor Author

(sorry folks, just fixing up a linting error!)

volodymyrZotov
volodymyrZotov previously approved these changes Dec 15, 2022
@kanadgupta kanadgupta requested review from SimonBarendse and removed request for DCKcode December 15, 2022 15:35
DCKcode
DCKcode previously approved these changes Dec 15, 2022
@kanadgupta
Copy link
Contributor Author

So sorry y'all, I was playing around locally and realized that it doesn't make sense for us to include whoami in our list of commands. This is officially ready to be merged! 😅

@SimonBarendse SimonBarendse merged commit 92aaff8 into 1Password:main Dec 16, 2022
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.

New plugin: ReadMe CLI (a.k.a. rdme)
5 participants