Closed aisrael closed 3 months ago
Thanks for this recommendation and implementation, @aisrael! We'll take a look and provide any necessary feedback.
This is looking good from my testing! Setting secrets with secrets set
and via the tui
are working as expected. @aisrael I would recommend the following change though:
diff --git a/pkg/cmd/secrets.go b/pkg/cmd/secrets.go
index 342fa8c..3a04d16 100644
--- a/pkg/cmd/secrets.go
+++ b/pkg/cmd/secrets.go
@@ -237,6 +237,7 @@ func setSecrets(cmd *cobra.Command, args []string) {
canPromptUser := !utils.GetBoolFlag(cmd, "no-interactive")
localConfig := configuration.LocalConfig(cmd)
visibility := cmd.Flag("visibility").Value.String()
+ visibilityModified := visibility != ""
utils.RequireValue("token", localConfig.Token.Value)
@@ -314,7 +315,7 @@ func setSecrets(cmd *cobra.Command, args []string) {
Name: key,
Value: &value,
}
- if visibility != "" {
+ if visibilityModified {
changeRequest.Visibility = &visibility
}
changeRequests = append(changeRequests, changeRequest)
@@ -327,7 +328,7 @@ func setSecrets(cmd *cobra.Command, args []string) {
Name: key,
Value: &value,
}
- if visibility != "" {
+ if visibilityModified {
changeRequest.Visibility = &visibility
}
changeRequests = append(changeRequests, changeRequest)
@@ -347,7 +348,7 @@ func setSecrets(cmd *cobra.Command, args []string) {
} else {
changeRequest.Value = &secretArr[1]
}
- if visibility != "" {
+ if visibilityModified {
changeRequest.Visibility = &visibility
}
changeRequests = append(changeRequests, changeRequest)
@@ -360,7 +361,7 @@ func setSecrets(cmd *cobra.Command, args []string) {
}
if !utils.Silent {
- printer.Secrets(response, keys, jsonFlag, false, raw, false, false)
+ printer.Secrets(response, keys, jsonFlag, false, raw, false, visibilityModified)
}
}
This will make it so if the visibility is adjusted the table that gets printed afterward will include the visibility values as well.
@watsonian Made the changes as suggested.
@aisrael Awesome! We'll need another engineer on our CLI team to review before approving for merge, but it's looking good!
@aisrael This is looking great! It looks like we had a failing test as a result of an earlier change. Would you mind rebasing your branch on DopplerHQ:master
?
@nmanoogian Just updated my fork/branch from DopplerHQ/cli:master
@aisrael Excellent! Sorry to do this but one more ask that I forgot about. Can you prepend chore:
to your "Show visibility" commit message (similar to https://github.com/DopplerHQ/cli/pull/456/commits/3f12e954bde4944e037de30be9769a3214d84ab1). We autogenerate our release notes via the commit message and prepending chore:
keeps the noise down.
So... should I just push a 'dummy' commit in my fork branch with chore:
?
@aisrael I believe all Nic is looking for is a git rebase -i
where you adjust the commit message on your initial commit (you can get rid of the "dummy" commit you made). In fact, you could probably just roll all your commits into a single message and then force-push it up on this branch.
Got it, ok will try
Why was this pull request opened?
Submitting this as a proof-of-concept and potential implementation for [FEATURE] Set secret visibility #453
What was changed?
model.ChangeRequest
instead of using amap[string]interface{}
model.ChangeRequest
to add all fields in the secrets-update APIHow to test?