Result of nixpkgs-review pr 106928 1

@GrahamcOfBorg eval

/status needs_reviewer

/marvin opt-in
/status needs_reviewer

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

lgtm

/status needs_merger

@ofborg build sonobuoy

LGTM, thank you! I've just added to the PR a few comments to make reviewing the next upgrade easier, as I had been surprised by why there was a need for rev despite the version field being present :)

Hmm well with us having no darwin builders, I guess triggering an ofborg build before ofborg eval finishes leads to a red cross being indefinitely there. Let's land anyway, thank you for your contributions!

thanks folks! I'm fairly new to writing Nix and not much functional language experience either, so I'd appreciate any suggestions you have for this to be easier / cleaner!

The only idea I'd have to make this cleaner / easier to update would be, to not have to hard-code the rev in -X ${t}/pkg/buildinfo.GitSHA=${rev}.

Which could probably be done by using git rev-parse ${version}, if you still have .git, which you could have once #107322 (review) lands with fetchFromGitHub (you'll need to fall back to fetchgit in the meantime).

That said, it might also make sense to just set the rev there as full-zeros, depending on what it's exactly used for — if it's only used to show it back to the user, then I feel like it's of little importance.

What do you think?

I can see the argument for full-zeros on the rev, but the product as is today has plenty of updates and I think there are values to keep the rev:

• minimize myself, as a maintainer, in making mistakes when sending updates to publish
• accidents in which (hopefully will never happen!) the git tag unexpectedly points to a different rev

More or less to hold myself accountable and provide transparency to users.

Appreciate the pointers! I'll be looking forward to the rev-parse feature to be available on fetchFromGitHub 😄