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.

Sign up for GitHub

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

Jump to bottom

Added Vertica Shell Plugin (vsql) #327

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Added Vertica Shell Plugin (vsql) #327

merged 2 commits into from
Sep 19, 2023

Conversation

parthiv11
Copy link
Contributor

Overview

Vertica Shell Plugin added

  • Created a new plugin
  • Improved an existing plugin
  • Fixed a bug in an existing plugin
  • Improved contributor utilities or experience

Related Issue(s)

  • Resolves: #
  • Relates: #

How To Test

Changelog

Additional information

AndyTitu
AndyTitu reviewed Jun 29, 2023
View reviewed changes
Copy link
Contributor

@AndyTitu AndyTitu left a comment

Choose a reason for hiding this comment

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

Did a first iteration and left a few comments

plugins/vertica/database_credentials.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials_test.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials.go Show resolved Hide resolved
@techcraver
Copy link
Contributor

Hello! Don't forget to also write an accompanying blog post on Hashnode with the tags 1Password and BuildWith1Password (not just putting #1Password in the text - but use the Hashnode tags in the CMS. :)

The full instructions are here.

@arunsathiya arunsathiya added waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member hashnode hackathon Ideas and inspiration for the hackathon running from June 1st - June 30th labels Jun 29, 2023
@arunsathiya arunsathiya changed the title 😎Added Vertica Shell Plugin (vsql) Added Vertica Shell Plugin (vsql) Jun 29, 2023
hculea
hculea reviewed Jun 30, 2023
View reviewed changes
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! ❤️

plugins/vertica/database_credentials_test.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials.go Show resolved Hide resolved
plugins/vertica/database_credentials_test.go Show resolved Hide resolved
hculea
hculea reviewed Jul 4, 2023
View reviewed changes
plugins/vertica/database_credentials.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials.go Outdated Show resolved Hide resolved
plugins/vertica/database_credentials.go Show resolved Hide resolved
@hculea hculea requested a review from AndyTitu July 5, 2023 07:34
@parthiv11
Copy link
Contributor Author

Is there anything left in vsql plugin?

AndyTitu
AndyTitu approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@AndyTitu AndyTitu left a comment

Choose a reason for hiding this comment

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

@parthiv11 This PR looks ready to me! I'll approve proactively but there are 2 more points I'd like to tick off before merging:

  • fix linter by formatting file /vertica/database_credentials.go with gofmt\
  • provide a video of this plugin working as expected as you did for CrateDB

@parthiv11
Copy link
Contributor Author

vertica.mp4

hculea
hculea approved these changes Jul 17, 2023
View reviewed changes
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! Only 1 nit.

plugins/vertica/database_credentials.go Outdated Show resolved Hide resolved
@hculea hculea added waiting-on-sec-review and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels Jul 17, 2023
SimonBarendse
SimonBarendse approved these changes Jul 25, 2023
View reviewed changes
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.

Thank you for your contribution!

@arunsathiya
Copy link
Contributor

@parthiv11 It just came to my notice that your commits are not signed. Could you sign all of them (easy with 1Password) and amend the past commits? This guide may help.

@parthiv11 parthiv11 force-pushed the vsql branch 9 times, most recently from b90c558 to e576bea Compare July 31, 2023 05:55
@parthiv11
Copy link
Contributor Author

@arunsathiya what next?

AndyTitu
AndyTitu approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@AndyTitu AndyTitu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

re-approaving

@parthiv11 could you please sign all your commits so we can merge?

@AndyTitu AndyTitu added the in-progress this PR is being worked on/comments are in the process of being addressed by the contributor label Aug 30, 2023
@parthiv11
Copy link
Contributor Author

@AndyTitu please check out ..

@hculea hculea merged commit a4de470 into 1Password:main Sep 19, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hculea hculea hculea approved these changes

@AndyTitu AndyTitu AndyTitu approved these changes

@SimonBarendse SimonBarendse SimonBarendse approved these changes

Assignees

@parthiv11 parthiv11

Labels
hashnode hackathon Ideas and inspiration for the hackathon running from June 1st - June 30th in-progress this PR is being worked on/comments are in the process of being addressed by the contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@parthiv11 @techcraver @arunsathiya @SimonBarendse @AndyTitu @hculea