-
Notifications
You must be signed in to change notification settings - Fork 206
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
Change secrets Replace to modify the ID #1532
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Angry linter but otherwise LGTM |
/lgtm |
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.
Let's stick to the 2 LGTM rule please. No need to rush things through so close to a release.
Test isn't happy yet
|
We decided that podman secret create --replace should match behaviour of podman container create --replace, so the ID should change. Signed-off-by: Daniel J Walsh <[email protected]>
@vrothberg PTAL |
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.
Code LGTM
Can we rename Replace
to Update
?
I think in this case it's replace, where the ID is modified. If the secret ID is not modified, then update is the correct verbiage. |
LGTM for replace, we can look at update in a separate patch if needed later on. /lgtm |
/hold cancel |
We decided that podman secret create --replace should match behaviour of podman container create --replace, so the ID should change.