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

fix(new-plugin): Set only the last word of the credential name as the field name, if it's larger than seven characters #263

Merged
merged 4 commits into from Sep 22, 2023

Conversation

arunsathiya
Copy link
Contributor

Overview

Prior to this change, when the credential name's last word is larger than seven characters, the field name generated by the "make new-plugin" template is empty.

With this change, the last word of the credential name is set as the field name. In the example below, field name Credentials is generated for the credential name GitHub Access Credentials.

image

If the last word is smaller than seven characters, the usual pattern of stitching together the last several words of the credential name (proposed originally on this PR #72) will be retained. In the example below, field name API Key is generated for the credential name GitHub API Key.

image

Type of change

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

Related Issue(s)

How To Test

  • Clone this branch and switch to it.
  • Run make new-plugin
  • When prompted to enter a credential name, try two variants: one with the last word less than seven characters, and one with the last word having greater than seven characters. See that both work as expected, especially the latter variant should have only the last word as the field name.

Changelog

Set only the last word of the credential name as the field name, if it's larger than seven characters

@arunsathiya arunsathiya added the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label May 22, 2023
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.

Thanks for this very well documented improvement! Lgtm

cmd/contrib/main.go Outdated Show resolved Hide resolved
cmd/contrib/main.go Outdated Show resolved Hide resolved
cmd/contrib/main.go Outdated Show resolved Hide resolved
@arunsathiya arunsathiya self-assigned this May 25, 2023
@accraw accraw added in-progress this PR is being worked on/comments are in the process of being addressed by the contributor and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels May 29, 2023
@arunsathiya arunsathiya added waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member and removed in-progress this PR is being worked on/comments are in the process of being addressed by the contributor labels Jun 27, 2023
@arunsathiya
Copy link
Contributor Author

@AndyTitu and @hculea, this is ready for another look now. Thank you!

Copy link
Member

@jpcoenen jpcoenen 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 improving the contribution framework! Improvements like these make our work so much easier in the long-term 😀

cmd/contrib/main.go Show resolved Hide resolved
cmd/contrib/main.go Outdated Show resolved Hide resolved
@accraw accraw added in-progress this PR is being worked on/comments are in the process of being addressed by the contributor and removed waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member labels Jun 28, 2023
@hculea hculea removed their request for review July 5, 2023 08:07
Copy link
Member

@jpcoenen jpcoenen left a comment

Choose a reason for hiding this comment

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

One tiny remark, but pretty much good to go 🚀

cmd/contrib/main.go Outdated Show resolved Hide resolved
cmd/contrib/main_test.go Outdated Show resolved Hide resolved
… it's clearer while reading the code, and prepend the rest of the words from credNameSplit, working backwards. More specifically, we also check if the second to the last word and the last word included are still lesser than the cutoff and use the second to the last word too, instead of just using the last word
@arunsathiya
Copy link
Contributor Author

Rebased with main to fix conflicts.

@arunsathiya arunsathiya added waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member and removed in-progress this PR is being worked on/comments are in the process of being addressed by the contributor labels Aug 17, 2023
@SimonBarendse SimonBarendse merged commit 8f71c92 into 1Password:main Sep 22, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member
Projects
None yet
6 participants