-
Notifications
You must be signed in to change notification settings - Fork 18
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
Implement freezing and thawing of pins #78
base: master
Are you sure you want to change the base?
Conversation
This allows freezing a pin to ensure that it isn't changed by issuing a blanked `npins update`.
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.
Seems to work as expected. Thanks for looking into this!
help Prints this message or the help of the given subcommand(s) | ||
import-flake Try to import entries from flake.lock | ||
import-niv Try to import entries from Niv | ||
init Intializes the npins directory. Running this multiple times will restore/upgrade the | ||
`default.nix` and never touch your sources.json | ||
remove Removes one pin entry | ||
show Lists the current pin entries | ||
thaw Thaw a pin entry |
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 get that logically freeze
<-> thaw
makes sense but from a UX perspective I would prefer to have a more explicit indication that those two commands belong together.
Hence I'd prefer freeze
and unfreeze
- WDYT @andir ?
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'd also prefer unfreeze
, with an optional alias to thaw
for those who find that more intuitive
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.
Looking good, only some nits (and "thaw" as name)
@@ -123,14 +123,16 @@ macro_rules! mkPin { | |||
version: Option<<$input_name as Updatable>::Version>, | |||
#[serde(flatten)] | |||
hashes: Option<<$input_name as Updatable>::Hashes>, | |||
#[serde(default)] | |||
frozen: Frozen, |
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.
Why doesn't our CI/rustfmt complain on this line? (Mixed usage of tabs and spaces)
@@ -589,6 +602,11 @@ impl Opts { | |||
|
|||
if opts.names.is_empty() { | |||
for (name, pin) in pins.pins.iter_mut() { | |||
if pin.is_frozen() { | |||
log::info!("Skipping pin {} as it is frozen.", name); |
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.
Nit for consistency:
log::info!("Skipping pin {} as it is frozen.", name); | |
log::info!("Skipping pin '{}' as it is frozen.", name); |
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.
Yeah, I was planning on doing this for the entire code base. We've a few cases where that isn't the case.
Now that I actually think of it, what is the workflow of manually updating frozen pins? Unfreezing them just for that purpose seems a bit unwieldy. |
Perhaps an extra argument to the |
This allows freezing a pin to ensure that it isn't changed by issuing a blanked
npins update
.Should fix #62