-
Notifications
You must be signed in to change notification settings - Fork 133
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
Add changelog #4448
base: master
Are you sure you want to change the base?
Add changelog #4448
Conversation
Thank you for making a first draft as basis for discussions. My two cents: I am not sure if having a CI check which flags PRs that don't modify Moreover, it leaves open the question who really is responsible for adding entries to CHANGELOG.md For sake of discussion, I'll describe an alternative model used by GAP at https://github.com/gap-system/gap which revolves around labels for PRs. In GAP we use these labels:
A couple other labels are used to indicate in which category the release notes fit (so in the context of this PR these might correspond to "Added", "Changed", "Deprecated", "Removed", etc.; but one could also use other categories, either instead or in addition to ehse, for example based on feature domain: "Algebraic Geometry", "Polyhedral Geometry", "Lie Theory", "Number theory", "Group Theory", etc.) "Ordinary" contributors don't have to interact with these at all (indeed if you are not a project member, you can't set labels). Instead experienced developers may add these labels to PRs. A key point is that this can happen at any time: during creation of a PR, while it is open, or even after it is merged. In GAP we then have a script which gets as input the new version and which queries all PRs merged since the last release, and based on the labels updates the release notes / changelog. PRs which don't have the required label are put into a separate section "uncategorized". Then someone tasked with updating the release notes (e.g. release managers) can occasionally run this script and look into PRs w/o release notes labels, and deal with them: e.g. by requesting that the PR author provide a succinct summary suitable for the release notes, which then is put into the PR title and a label In GAP this works quite well because it avoids merged conflicts; and decouples adding new work from writing the changelog entry, which reduces the barrier for contribution; while making it possible to still keep track of what's changed in a systematic way so nothing is accidentally "forgotten". Of course in the end someone has to write a summary for a PR; that is additional work we can't quite eliminate if we want useful release notes. But we can still strive to make it as painless as possible. |
We discussed this in triage today and agreed to try my suggestion of working with labels (of course we can and will evaluate & revise this based on how it works in practice). As such the workflow being added here should be removed. One good suggestion by @HereAround was to set up another CI job which runs the scripting updating I also am not yet sure if the suggested categories in In the meantime, @aaruni96 can you please make similar PRs for AA and Nemo? |
Could someone also add instructions to the developer docs? I think I understand how the system is supposed to work, but I always like to read up on things :) |
[skip-ci]
Removed the changelog workflow. But the labels got me thinking, should we consider a default PR template, which adds a label like Documentation for PR template : https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository |
Yes, we definitely need to document in the dev docs how the changelog update workflow is supposed to work, and how to use those labels. Perhaps the description we have for GAP can be used as a starting point. Regarding PR template and labels: we don't need a But a template still would be a good idea, namely to suggest that people should provide a release note draft (and how), and possible info on how to set labels (but phrased in a way that makes it clear that this only applies to people who actually can set labels; i.e. don't confuse newbies who may frantically search for how they can add labels when they simply can't). E.g. this is what we use for GAP: https://github.com/gap-system/gap/blob/master/.github/pull_request_template.md The Python script we use in GAP for updating release notes is here: https://github.com/gap-system/gap/blob/master/dev/releases/release_notes.py It uses the Aaanyway, it should be possible to relatively quickly modify the script for us. And then improve it: right now it produces a new file, and then the release manager (for GAP that is me) inserts it into the real changelog manually. But this could easily be automated, too, to insert the new text into the "right" spot. |
|
||
### Security |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Security |
I don't think this is a category that is relevant for us... or if it ever becomes, we can add it :-)
On the other hand, I wonder how breaking changes fit into this scheme: right now "breaking" could appear under "changed" or "removed" (and arguably even under "fixed"). But we should make breaking changes prominent. I see two options:
- Add another section "Breaking" (I'd put it first) and put them all in there, no matter whether they remove or change something
- Use these categories but then always put Breaking: in front of breaking changes
(As @thofma pointed out for OSCAR this shouldn't be relevant for some time, but certainly for AA and Nemo. But I am asking this here in the hopes that more people will see it this way).
This PR adds a changelog file to Oscar. The changelog should include all noteworthy changes to Oscar. This also adds a changelog CI check to every PR, but this will not be a required check, just a reminder.
The template is borrowed from https://keepachangelog.com/en/1.1.0/.
Addresses #4431 .