treewide: migrate rev = "refs/tags/..." to tag#414087
treewide: migrate rev = "refs/tags/..." to tag#414087pbsds wants to merge 1 commit intoNixOS:masterfrom
rev = "refs/tags/..." to tag#414087Conversation
| description = "Lint commit messages with conventional commit messages"; | ||
| homepage = "https://keisukeyamashita.github.io/commitlint-rs"; | ||
| changelog = "https://github.com/KeisukeYamashita/commitlint-rs/releases/tag/${src.rev}"; | ||
| changelog = "https://github.com/KeisukeYamashita/commitlint-rs/releases/tag/v${version}"; |
There was a problem hiding this comment.
| changelog = "https://github.com/KeisukeYamashita/commitlint-rs/releases/tag/v${version}"; | |
| changelog = "https://github.com/KeisukeYamashita/commitlint-rs/releases/tag/${src.tag}"; |
or src.rev if you want. Otherwise this will 404 if the tag format changes.
There was a problem hiding this comment.
- see Using
finalAttrs.src.tagbreaks ifsrcis overriden to userev#367739 - and treewide: remove
refs/tags/from github releasemeta.changelogurls #338301
In #368177 we ended up going with this approach for being simpler and avoiding most pitfalls.
There was a problem hiding this comment.
If you're worried about that use src.rev which is always available.
But the biggest pitfall is the tag format changing.
There was a problem hiding this comment.
Note that this expression doesn't even use finalAttrs.
There was a problem hiding this comment.
I treat all src.* and pname as if they have finalAttrs, since any derivation is one PR away from migrating to it.
There was a problem hiding this comment.
Sure, but then the changelog URL is most likely broken anyway and it's actually good that it breaks loudly.
There was a problem hiding this comment.
to my understanding nix-update migrates to src.tag, which is a slow mass-migration in the other direction of this and the prior PR. I think we should come to a treewide preference on this one.
There was a problem hiding this comment.
I agree and I have strong preference for something that doesn't require changing the changelog URL whenever the tag format changes, especially since @r-ryantm does the latter automatically.
There was a problem hiding this comment.
Anyway, since there is no consensus this PR has no right to make a treewide change.
EDIT: I didn't realize the PR only fixes instances where the changelog is already broken.
There was a problem hiding this comment.
I agree and I have strong preference for something that doesn't require changing the changelog URL whenever the tag format changes, especially since @r-ryantm does the latter automatically.
Unsurprisingly I also agree and want a treewide preference. The tag format changing happens far less than I expected.
Maybe the point of leverage is the update script and a test inside of that, at the time that the script was run, the change log did not 404.
|
Appreciate you, Peder. Get that degree! |
Redo of #368177, part of #346453.
I've modernized the script with what i've learned during other treewides in preparation for adapting it for non
"refs/tags/..."migrations ofrev.Done with:
and filtered with
GIT_DIFF_OPTS='--unified=20' git -c interactive.singleKey=true add --patch.I the checked
diff -u packages{,2}.jsonwhich only shows changedmeta.changelogs and the nixos-manual, hence no eval failures (which CI does not report on) are introduced.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.