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

generate: rename --workload-owner-key to --add-workload-owner-key #670

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

Freax13
Copy link
Contributor

@Freax13 Freax13 commented Jul 3, 2024

This makes it more obvious to the user that this flag only ever adds keys and never removes existing keys.

This change was made following the discussion in #650 (review).

@Freax13 Freax13 added the breaking change A user-affecting breaking change label Jul 3, 2024
@Freax13 Freax13 requested a review from burgerdev July 3, 2024 11:33
@Freax13 Freax13 requested a review from katexochen as a code owner July 3, 2024 11:33
Comment on lines 66 to 67
cmd.Flags().StringArrayP("add-workload-owner-key", "w", []string{workloadOwnerPEM},
"path to workload owner key (.pem) file (can be passed more than once)")
Copy link
Contributor

Choose a reason for hiding this comment

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

The docstring should describe the behavior. Maybe

add a workload owner key from a PEM file to the manifest (pass more than once to add multiple keys)

This makes it more obvious to the user that this flag only ever adds
keys and never removes existing keys.
@Freax13 Freax13 force-pushed the tom/workload-owner-key-rename branch from 2c54f56 to db38134 Compare July 4, 2024 08:21
@Freax13 Freax13 merged commit 5764304 into main Jul 4, 2024
6 of 7 checks passed
@Freax13 Freax13 deleted the tom/workload-owner-key-rename branch July 4, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change A user-affecting breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants