Skip to content

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
SimonBarendse merged 4 commits into
1Password:mainfrom
arunsathiya:fix/164-missing-field-name
Sep 22, 2023
Merged

fix(new-plugin): Set only the last word of the credential name as the field name, if it's larger than seven characters#263
SimonBarendse merged 4 commits into
1Password:mainfrom
arunsathiya:fix/164-missing-field-name

Conversation

@arunsathiya

Copy link
Copy Markdown
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

@AndyTitu AndyTitu left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread cmd/contrib/main.go Outdated
Comment thread cmd/contrib/main.go Outdated
Comment thread cmd/contrib/main.go Outdated
@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 arunsathiya requested review from AndyTitu and hculea June 27, 2023 05:31
@arunsathiya

Copy link
Copy Markdown
Contributor Author

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

@jpcoenen jpcoenen left a comment

Copy link
Copy Markdown
Member

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 😀

Comment thread cmd/contrib/main.go
Comment thread cmd/contrib/main.go Outdated
@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

@jpcoenen jpcoenen left a comment

Copy link
Copy Markdown
Member

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 🚀

Comment thread cmd/contrib/main.go
Comment thread cmd/contrib/main_test.go Outdated
… 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 arunsathiya force-pushed the fix/164-missing-field-name branch from d7791d8 to 59a5c55 Compare August 17, 2023 10:24
@arunsathiya

Copy link
Copy Markdown
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
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

Development

Successfully merging this pull request may close these issues.

Missing field name in the generated template when last word of the Credential Name is greater than 7 characters

6 participants