-
Notifications
You must be signed in to change notification settings - Fork 172
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 support for Civo, a managed Kubernetes service #296
Conversation
Signed-off-by: Cheithanya <cheithanya2002@gmail.com>
As noted on the other comment, we'd like to see the "Overview" section of the PR description filled 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.
Thanks for the contribution.
Left a few suggestions and a question.
@arunsathiya @hculea you can review it now |
@itsCheithanya I see there is a another contribution for the same plugin: #312 authored by @siddhikhapare. To deduplicate some of the remaining work and help move this forward faster, I'd love if you two could collaborate to get this to the finish line 😄 Regardless of which PR you will choose to proceed with, we will be counting this contribution for both of you for the hackathon. Please let me know if that is okay with you and whether I could help you two connect, on Discord or the 1Password Developer Workspace on Slack! Let's go 🚀 |
Hi @hculea , thank you for reaching out. It would great to collaborate with @siddhikhapare and add the missing parts to avoid duplication I wanted to inform you that @AndyTitu and I are already working on the plugin to bring it to completion. Therefore, it would be great if we could use my pull request for further development since contribution for both of us will be counted for the hackathon. |
It's fine with me but we'll also have to confirm with @siddhikhapare that it's okay to close their PR as duplicate and for you to focus your efforts together on this PR |
@AndyTitu @hculea @itsCheithanya I closed my PR. I'm looking forward to working together. |
@hculea @AndyTitu @arunsathiya Done |
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.
Code looks great! @arunsathiya can I ask you to take it for a functional spin?
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.
Thanks for your submission, @itsCheithanya and @siddhikhapare. I am unclear on what's happening with CIVO_API_KEY
and CIVO_TOKEN
though, and also on what's happening in the config file importer. Could you address those parts and other nits throughout the PR?
We'll be around if you have any questions along the way.
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.
@siddhikhapare this PR seems ready, we just have a few very minor suggestions, but I am approving this. If you could address the last suggestions, we could merge this PR. Thanks!
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.
Appreciate the contribution, @itsCheithanya! ❤️
Overview
Type of change
Related Issue(s)
How To Test
Install Civo CLI
https://www.civo.com/docs/overview/civo-cli
Test the plugin by running
op plugin init civo
civo region ls
Changelog