Skip to content
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

Jump to bottom

Changelog meta entry #60371

Merged
merged 4 commits into from May 10, 2019
Merged

Changelog meta entry #60371

merged 4 commits into from May 10, 2019

Conversation

7c6f434c
Copy link
Member

@7c6f434c 7c6f434c commented Apr 28, 2019 •
edited

Motivation for this change

There are various calls to pay attention to changelog. For easier location of the changelog and for better tooling support, we should allow putting a link to the changelog into meta.

This only changes the documentation and the meta correctness check.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • N/A Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@7c6f434c 7c6f434c requested review from grahamc and ryantm April 28, 2019 12:53
grahamc
grahamc approved these changes Apr 28, 2019
View reviewed changes
Copy link
Member

@grahamc grahamc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea.

Can you add a changelog entry to a single package in this PR as well, to ensure ofborg gets along with it?

@7c6f434c
Copy link
Member Author

Given that the example in the documentation already works, added it to hello itself, too.

@c0bw3b
Copy link
Contributor

c0bw3b commented Apr 28, 2019

There was a related discussion in #50483

@7c6f434c
Copy link
Member Author

Hm, a good question: should we allow null for lack of changelog, or should it be empty list, or just a comment?

ryantm
ryantm requested changes Apr 28, 2019
View reviewed changes
Copy link
Member

@ryantm ryantm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nixpkgs-update can use this to provide a link to the changelog. I'm not sure how much this will help to get the text of the changes into the report. It would depend on how easy it is to parse changelogs, or if it is good enough to include say the first 100 lines of the changelog file.

pkgs/applications/misc/hello/default.nix Outdated Show resolved Hide resolved
doc/meta.xml Outdated Show resolved Hide resolved
ryantm and others added 2 commits April 28, 2019 16:52
@ryantm @7c6f434c
Switch to plaintext changelog link
4f72cbe 
Co-Authored-By: 7c6f434c <7c6f434c@mail.ru>
@ryantm @7c6f434c
Switch to plaintext changelog link in the example.
7f1a66b 
Co-Authored-By: 7c6f434c <7c6f434c@mail.ru>
@7c6f434c
Copy link
Member Author

  1. Thanks for the plaintext link suggestion — I found something that looks mostly OK and stopped there, plaintext is indeed better.

  2. I think a link to the changelog will be quite useful. 100 first lines sounds like it could be useful more often than not, but it might be suboptimal in case only HTML page (with header etc.) is available.

@c0bw3b
Copy link
Contributor

c0bw3b commented Apr 28, 2019

Hm, a good question: should we allow null for lack of changelog, or should it be empty list, or just a comment?

I guess an empty string (or empty list) would be expected when no changelog is given.
Similarly to not setting any homepage or any description.

@7c6f434c
Copy link
Member Author

@c0bw3b I do not think there has been a push to set meta.homepage for a maximum possible number of packages; it looks like some people could try such an effort for meta.changelog, so I wonder how to distinguish «there is no such thing» and «we haven't looked for it yet» in a way that is friendly to tooling.

@ryantm
Copy link
Member

ryantm commented Apr 28, 2019

I approve the changes; of course, the commit messages should get fixed up before merging.

@7c6f434c
Copy link
Member Author

My current plan is indeed to squash-merge.

@grahamc
Copy link
Member

grahamc commented Apr 28, 2019 via email

@7c6f434c
Copy link
Member Author

Hopefully license always exists (otherwise it is unfree by definition), but homepage can fail to exist and maybe there should be a documented way to make a claim it doesn't exist. I think empty list is better than empty string; no opinion on allowing null instead of [].

@7c6f434c
Copy link
Member Author

7c6f434c commented May 1, 2019

OK, the current plan us to assume that as homepage and changelog can be lists of alternatives, [] means that we looked for a page but found nothing. Right now there is no need to document it; once some tooling starts demanding these attributes, we will document it in a place that makes the most sense in the new context.

(The current PR will be squash-merged as-is)

@7c6f434c 7c6f434c merged commit 76e2a96 into NixOS:master May 10, 2019
tadeokondrak pushed a commit to tadeokondrak/nixpkgs that referenced this pull request May 17, 2019
@7c6f434c @tadeokondrak
Changelog meta entry (NixOS#60371)
033d25a 
meta.changelog: enable, document, add for GNU Hello
This was referenced Nov 8, 2019
Open
Closed
@timokau
Copy link
Member

timokau commented Dec 11, 2019

I just tried to add a changelog to ntl. The changelog is available here. However linking to that page has two disadvantages:

  • its not possible to directly link to the current version
  • there is no guarantee that the page will be available in the future

The changelog is also available in $src/doc/tour-changes.html or $out/share/doc/NTL/tour-changes.html. It would be much nicer to link to one of those instead. We already mirror the source, so availability guarantees are much better. Also the relevant version will always be at the top.

However as far as I understand pointing to $out is problematic in a meta entry, since it requires a build in order to evaluate. Pointing to src is also problematic, even if a bit less so.

Opinions?

@7c6f434c
Copy link
Member Author

I think that meta.changelog is either for humans or for tools processing fresh changes.

Therefore I would set the changelog reference to a list, with the first one being the URL to the online version, and the second being a literal string ''$src/doc/tour-changes.html'', which is easy for humans to understand and will not be reached by tools as long as the first one is up.

Maybe "\${src}/doc/tour-changes.html" is a bit better fallback, no opinion here.

cdepillabout added a commit to cdepillabout/nixpkgs that referenced this pull request Apr 9, 2021
@cdepillabout
haskellPackages: add changelog argument to Haskell generic-builder.nix
f136f19 
changelog was recently added as a new meta field in
NixOS#60371.

This commit adds similar support to the Haskell generic builder.
@cdepillabout cdepillabout mentioned this pull request Apr 9, 2021
Merged
10 tasks
sternenseemann pushed a commit that referenced this pull request Apr 9, 2021
@cdepillabout @sternenseemann
haskellPackages: add changelog argument to Haskell generic-builder.nix
c8ac41f 
changelog was recently added as a new meta field in
#60371.

This commit adds similar support to the Haskell generic builder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryantm ryantm ryantm approved these changes

@matthewbauer matthewbauer matthewbauer approved these changes

@grahamc grahamc grahamc approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

Assignees
No one assigned
Labels
6.topic: stdenv Standard environment 8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@7c6f434c @c0bw3b @ryantm @grahamc @timokau @matthewbauer