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

Add overwrite option to pws set command #157

Closed
wants to merge 1 commit into from

Conversation

robinkrahl
Copy link
Collaborator

This patch adds the --overwrite option to the pws set command and
changes the default behavior to refuse overwriting programmed slots.


I originally suggested this change in #139 but I think I’ve changed my mind: This form of the pws set command is pretty useless. If I want to overwrite a slot I should rather use pws update. If I want to add a new slot I most likely want to use pws add. Only if I want to set a specific slot I need pws set.

At the same time, changing the default behavior is a already breaking change. Why not remove pws set completely while at it and add a --slot option to pws add if I really want to write a specific slot?

This patch adds the --overwrite option to the pws set command and
changes the default behavior to refuse overwriting programmed slots.
@d-e-s-o
Copy link
Owner

d-e-s-o commented Apr 17, 2021

Yeah, I agree. Let's just remove pws set. I like the --slot suggestion.

@robinkrahl
Copy link
Collaborator Author

Do you want to remove pws set at once, or first deprecate it and remove it later?

@robinkrahl robinkrahl closed this Apr 18, 2021
@d-e-s-o
Copy link
Owner

d-e-s-o commented Apr 18, 2021

Do you want to remove pws set at once, or first deprecate it and remove it later?

I don't think I necessarily kept track of every breaking change in the past and we already discussed that basically "anything goes" before 1.0. So I am fine with just removing it straight away.

@robinkrahl robinkrahl deleted the pws-overwrite branch April 21, 2021 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants