-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
[Discussion] Should be changelog
be a static, non-parameterized link?
#336454
Comments
changelog
should be a static, non-parameterized link
It's not just that they're fragile. Linking to the changelog for a single version is less useful (and arguably less correct) than linking to the full changelog. Additionally, if the changelog does not get overwritten with the version (e.g. using |
I'd also suggest to change the examples in the wiki (such as this) if there is consensus. |
I think it’s more incorrect and misleading to show “future” changes that are not in the package version being looked up. |
I agree that it matters less for linking to changelog files in a repository using I also see the |
https://github.com/reflex-dev/reflex/releases/tag/v0.5.10 is not https://github.com/reflex-dev/reflex/releases/tag/refs/tags/v0.5.10 but r-ryantm/nix-update is prone to make that change |
In that case I don’t know what purpose the
FWIW, version‐specific changelogs are explicitly sanctioned by the manual:
|
@pbsds Fair point, that’s bad. I’m thinking of instances where we point directly to a changelog file in a repository, though. (Does anyone actually look at these changelog links? Are they surfaced in any prominent UI? Perhaps we should simply get rid of them.) |
I would be okay with that, but that should be it's own separate issue. |
The changelog links get added to commit messages and pr bodies when using nix-update. I usually check for breaking changes during backports and for critical services |
Well, I can see them having some utility if they encode the version like many existing examples do, like I said, but not so much if they are just a random static link. So they’re not unrelated matters in my eyes. Anyway I’m mostly just commenting so that people don’t prematurely assume there’s consensus about this and start bugging everyone to change it on their PRs, when even the manual’s example suggests doing it. Incorrect URLs should of course be fixed, though. |
Okay, but can we agree for now that following cases should definitely not be allowed:
|
I don’t really have an opinion about the releases URL. I think the main benefit – if there is one – is to be able to see what changed when a package updates. As a user I would probably get more benefit from the version‐specific release page. (But as I said, this whole bikeshed feels pretty pointless if we have no evidence that the changelogs are actually in any way useful to anyone. If we can figure out where – if anywhere – they are exposed, and how – if at all – people use them, then it might make sense to set policy based on what works best with those?) This just feels like another vector for #336200 that isn’t worth slapping the “tracking issue” label on at this point. The unstable versions problem is unfortunate, but |
changelog
should be a static, non-parameterized linkchangelog
be a static, non-parameterized link?
Another point to consider is that we often jump more than one upstream release in one update of Nixpkgs. The only possible way to accurately represent that (and unstable versions for that matter) in I don't think we should spend any significant time attempting to show this info to users. A link to the general upstream changelog is plenty as the user can reasonably be expected to find the relevant changes themselves on that page and even that only exists for convenience. In any case, do not go around asking people to change to one or the other. This is not what PR authors should be spending any amount of energy on. (And I don't think we should spend significant time discussing this either.) |
To give an example from a package I maintain, https://github.com/jsummers/imageworsener/blob/1.3.5/changelog.txt is a list of changes up to the version we package, so as long as you remember what version you were on previously it should list all the relevant changes. It’s true that GitHub releases make this hard‐to‐impossible to represent and that it’s somewhat hopeless for unstable versions, but I think the Of course “so long as you remember what version you were on previously” is doing a lot of heavy lifting here… and if you’re that engaged with a package you can probably already find the appropriate changelog yourself. Hence why I’m really tempted to say that this whole field (same as |
How can it be? Since removing things makes Nixpkgs smaller and arguably no functionality is lost, I am for deleting it - or making it deprecated and discouraged, given backwards-compatibility. (plus a treewide removal of them) |
I'm not in favor of dropping the urls, nor prescribing a lint/some review process to try and prepare for link breakage. If we don't care about downstream users who override the expression (and likely don't care about |
possibly of interest: #338301 |
r-ryantm comments the URL, if changelog is set, see e.g. #338318 and since r-ryantm is the most prolific source of PRs by a huge factor, at nearly double the number of commits of the most prolific human contributor, I'd consider it a common usecase I personally find it helpful for review, to identify if there's some potential runtime breakage (that can't be caught at buildtime), as well as to know whether a version bump can be backported |
That’s a good point! And having the changelog pinned to the relevant version is especially helpful in that case. I do wish we got full diff links too, though… |
However this is too dependent on the "forge". |
That can be solved by computing diffs from |
My 0,0294056 AUD: I'm maintaining a multiple-version package that uses upstream Wiki pages that detail the changes between each version:
Branch 4.19 will show the changes between 4.18 and 4.19, branch 4.18 will show the changes between 4.17 and 4.18, and so on. I imagined this would be more useful for people who are pinning their versions, as they know exactly what's new in that specific release. An alternative to this would be to use |
IMHO this problem manifests differently depending on where exactly the changelog is sourced from. If it's a |
Issue description
A short time ago @Pandapip1 noticed that changelogs with parameterized links are fragile: they can point out to incorrect URLs if e.g. they follow the Nixpkgs unstable schema.
The text was updated successfully, but these errors were encountered: