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

Render CLI Shell plugin #308

Closed
wants to merge 27 commits into from
Closed

Render CLI Shell plugin #308

wants to merge 27 commits into from

Conversation

smyja
Copy link

@smyja smyja commented Jun 22, 2023

Overview

Type of change

  • Created a new plugin
  • Improved contributor utilities or experience

How To Test

Changelog

@smyja smyja marked this pull request as ready for review June 24, 2023 07:18
Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

I left some guidance, I'd love if you could address the remaining TODOs, before the next round of review. 😄

plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key_test.go Show resolved Hide resolved
@AndyTitu AndyTitu added the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label Jun 26, 2023
Copy link
Member

@hculea hculea 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 addressing my suggestions!

I see that this submission currently contains two plugins: deta and render. Let's move the former in a separate PR.

plugins/render/api_key.go Outdated Show resolved Hide resolved
@techcraver
Copy link
Contributor

Hello! Is this a submission for the 1Password Hackathon with Hashnode? If so, when you're ready, please be sure you write a blog post on Hashnode to make your submission official. Full instructions are on the Hackathon page.

@smyja
Copy link
Author

smyja commented Jun 26, 2023

Hello! Is this a submission for the 1Password Hackathon with Hashnode? If so, when you're ready, please be sure you write a blog post on Hashnode to make your submission official. Full instructions are on the Hackathon page.

It's an entry, i want to write one article for all my contributions.

@arunsathiya arunsathiya added the hashnode hackathon Ideas and inspiration for the hackathon running from June 1st - June 30th label Jun 26, 2023
@smyja smyja requested a review from hculea June 28, 2023 05:35
Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Moving in the right direction! Love it 🚀

plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key_test.go Show resolved Hide resolved
plugins/render/plugin.go Outdated Show resolved Hide resolved
@smyja smyja requested a review from hculea June 28, 2023 10:30
Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

We're almost there! 🚀

plugins/render/render.go Outdated Show resolved Hide resolved
plugins/render/render.go Show resolved Hide resolved
plugins/render/api_key_test.go Outdated Show resolved Hide resolved
plugins/render/api_key.go Outdated Show resolved Hide resolved
plugins/render/api_key_test.go Outdated Show resolved Hide resolved
@smyja smyja requested a review from hculea June 29, 2023 09:39
Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Have a look over the test suite and lint - I left one indication over how to start. 😄

plugins/render/api_key_test.go Show resolved Hide resolved
@hculea
Copy link
Member

hculea commented Jun 29, 2023

Also, could you please sign your commits? The trace of the Github Action ("Check signed commits in PR") should provide a good indication of how to do so.

@hculea
Copy link
Member

hculea commented Jun 29, 2023

You can validate your tests to pass by running make test locally, before pushing your code 😄 It's faster than waiting for the pipelines to pass/fail!

Do let me know if you need any help to make the pipelines ✅ .

smyja added 13 commits June 29, 2023 13:05
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
smyja added 12 commits June 29, 2023 13:05
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
Signed-off-by: Smyja <akposlive59@gmail.com>
@smyja
Copy link
Author

smyja commented Jun 29, 2023

Also, could you please sign your commits? The trace of the Github Action ("Check signed commits in PR") should provide a good indication of how to do so.

I have signed the commits.

@smyja smyja requested a review from hculea June 29, 2023 12:26
Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Code looks great!

I did NOT functionally test this, as I'm having some trouble currently with my local builds, so I'd love if @arunsathiya or @AndyTitu could take a look here.

Approving for the code.

Files: map[string]sdk.OutputFile{
"~/.render/config.yaml": {
Contents: []byte(plugintest.LoadFixture(t, "config.yaml") + "\n"),
},
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a newline directly to the file, shall we?

@smyja
Copy link
Author

smyja commented Jun 30, 2023

Code looks great!

I did NOT functionally test this, as I'm having some trouble currently with my local builds, so I'd love if @arunsathiya or @AndyTitu could take a look here.

Approving for the code.

great, can you change the label status?

@hculea hculea requested a review from AndyTitu July 5, 2023 07:36
@hculea
Copy link
Member

hculea commented Jul 5, 2023

@smyja I'll leave it like this, until we get a functional testing too.

Then we can proceed to the security review, when I will be changing the label. 😄

@AndyTitu
Copy link
Contributor

AndyTitu commented Jul 5, 2023

@smyja could you possible provide a video/image in which you test that this plugin works as expected for the render cli?

The steps to test are:

  1. checkout this branch
  2. run make render/build
  3. run op plugin init render given that you have op cli installed
  4. run a render command that requires auth

@smyja smyja closed this Jul 13, 2023
@arunsathiya arunsathiya removed the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashnode hackathon Ideas and inspiration for the hackathon running from June 1st - June 30th
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants