Can we add that for updates, the reviewer should find the changelog(s) and add a link (if OP does not already do that), and check that there is nothing missing.
What’s the best way to correct a nixpkgs-update automatic PR? Submit a new PR? Recommend the changes in review?
Just “fork the PR” and create a new one. The bot won’t be offended 
Just mention the replaced PR in the description. I’m not sure if githubs “fixes #xyz” keyword works with PRs, but you might try that.
1 Like
Just mention the replaced PR in the description. I’m not sure if githubs “fixes #xyz” keyword works with PRs, but you might try that.
We have recently seen it work for «Closes #NNNNN» in an update-collecting PR, I think.
Right, I think GitHub will only pay attention to the closes #NNNN phrases in the body a PR’s first comment/message.
This is a great way to handle r-ryantm’s PRs. If you close its PRs before merging the other one, it might open a duplicate PR on the next run.
Fair point, there is documentation for lib.attrsets, but it seems for nothing else.
I take it the “rebuild count” is the number in the label? What does that actually indicate?
Edit: Does that mean the number of packages that will get rebuilt as a consequence of updating this one?
Yes, it is the total number of packages that will be rebuild due to the change.
1 Like
I have edited the OP to clear up broken vs platforms, mention how to fix bot PRs and explain more clearly that reviews should be on GitHub.
1 Like
What is the value in having a changelog link in the PR? I could see the value if it was somehow accessible to the user when updating, but without that I’m not convinced its worth it to ask reviewers to hunt down changelogs for every single package.
If the change is anything more than changing the version number and sha256, having access to the changelog can help provide context for the other changes being made. It also provides an easy way to learn about what the package itself is if you’re not already familiar with it.
I agree that this is very important. If I hadn’t read any of the changelog entries for package updates for software that I knew virtually nothing about, I wouldn’t have reviewed (sometimes merged) so confidently.
Most projects had some sort of “release notes” too if their wasn’t a changelog. And at a last resort I would use the “compare changes” feature that’s present in many places.
Changelogs are also a good help in figuring out whether the PR can or cannot be backported.
Indeed a reviewer should be determining whether a change needs a backport or not.
I was mostly thinking about r-ryantm PRs, otherwise the author should already include the link to changelog as @lilyball says above.
As mentioned by @worldofpeace, changelogs are essential to properly review PRs. I have reviewed dozens of PRs that did not link the changelog and upon finding them and reading them, I discovered that the author forgot to add/remove some dependency.
If there is no changelog link, I want to encourage reviewers to find it and share it. It will save other reviewers time too.
1 Like
My apologies if this is not the appropriate place to ask this, but what is the general policy in nixpkgs for library update PRs that break packages that depend on them, e.g. the libtorrent 1.2.0 update? Do you recommend that the library should be split to have multiple versions so the broken packages can use the old version? Or do you have some tag added or just close the PR? Or does it depend on the situation?
1 Like
Of course
Still, in general I’d go through these solutions in order of preference:
- look if there are new upstream versions of the broken packages that work with the new library; apply the updates if there is
- look if there are new upstream patches for the broken packages to make them compatible; apply the patches if there is
- create an upstream issue about it if there is not (not really a short term solution, but good open source etiquette)
- ask myself whether there is any urgency to the update or it may be better to just wait for the broken packages to add support for the new library. If there is no real urgency, you can ask in the “PRs ready for review” tags to apply a “wait for upstream” tag and ideally also revisit the issue after some time.
- create two versions of the library in nixpkgs and configure the broken packages to keep using the old one
2 Likes
I agree that changelogs are good, I’m just not entirely convinced if they are worth the work required to make them a hard requirement for every update PR. I think if we want to prioritize changelogs more (which is a good idea), we should
- add a meta attribute for them (that points to a listing of changelogs or something)
- shift the burden from the reviewers to the PR authors to link the changelog for the particular version
Instead of requiring reviewers (which we already do not have enough of) to do this highly repetitive work.
1 Like
Can we add to contributing.md something like:
- added changelog for package update?
1 Like