-
Notifications
You must be signed in to change notification settings - Fork 21
Conversation
We no longer have to check whether the flag is persistent, since the bug with env var names is no longer present due to other enhancements.
flag.Changed() is not set if the flag is set through an environment variable.
Remove caching behaviour, since it does not bring any value to flag creation.
For AWS service init, the description would be "AWS role <role-name>" without replacing the placeholder properly. Now it was fixed for both aws and gcp.
This reverts commit d27bbea.
This way it is clearer when what kind error is thrown and as an added bonus, we can rely more on the compiler to verify code correctness.
Fixup of previous commit
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.
I would say it's half-past-launch-time!
Because of the long list of commits that have gone back and forth, I think we should consider squashing these commits @florisvdg @SimonBarendse any opinions on this?
I'm usually not a big fan of squashing, as we may lose some context and motivation for a change. However, I can see how the current commit history of going back-and-forth may not be ideal to find the whole set of changes + motivation either. Therefore, I'd like to make an exception to my rule for this PR and suggest to squash the changes. The commit message of the squash should be very verbose and contain motivation for most of the changes. |
In my case all the files made by the IDE would show when I wanted to make a commit. Thanks to Simon's tip, this can be solved with a global gitignore to no longer have the same thing on other repos as well.
Here is a draft of the squash commit message. Commit: Switch the CLI library from KingPin to Cobra Description: The change to cobra is made because KingPin is no longer maintained, and Cobra comes with more enhancements than KingPin. The following list of key changes have been made in the codebase:
For a more detailed version of the changes in the codebase, below is a sqashed version of the commits:
|
Here is the new draft of the squash commit Commit: Switch the CLI library from KingPin to Cobra Description: The change to Cobra is made for two main reasons:
Also, Cobra comes with some other enhancements that make the CLI construction simpler and more consistent. The following list of key changes have been made in the codebase:
|
The main differences from the KingPin Version of the CLI:
secrethub read --version
would have been valid, in this version, the--version
flag can only be called on the root command, i.e.secrethub --version
.secrethub audit --output-format [tab][tab]
will autocomplete totable
orjson
).NoEnvVar
function fromenv.go
not removing the environment variable corresponding to the flag it was called on has been solved.-h
shorthand for help texts.secrethub repo