Changelog script: multiple changelog entries for a single PR #5279

aaruni96 · 2025-09-08T20:51:31Z

Resolves #4673

Requires #5271

Turns the following body into the even more following changelog entries :

---
## Release Notes
- Does abc {package: AbstractAlgebra}
- Does 123 {package: Nemo}
- Questions 42 {package: Singular}
---

#### Changes related to the package AbstractAlgebra


- [#5271](https://github.com/oscar-system/Oscar.jl/pull/5271) Does abc 

#### Changes related to the package Nemo


- [#5271](https://github.com/oscar-system/Oscar.jl/pull/5271) Does 123 

#### Changes related to the package Singular


- [#5271](https://github.com/oscar-system/Oscar.jl/pull/5271) Questions 42

fingolfin · 2025-09-10T14:12:26Z

I'll review this PR once PR #5271 has been merged and this one has been rebased, as then it's much easier to see what the additional changes in here are.

lgoettgens · 2025-09-11T17:29:47Z

Does this also allow to specify two levels of labels for each entry?

This should also be added to https://docs.oscar-system.org/dev/DeveloperDocumentation/changelog/.

Please talk about how to use this with Martin before merging (no need to ping them at this point though) as they are one of the few people using this already extensively. Thus it would be good to get their feedback on usability.

aaruni96 · 2025-09-12T08:37:11Z

Does this also allow to specify two levels of labels for each entry?

Implementation wise, it replaces the original PR with multiple PRs in the PR list, with the only difference being different topics are added to these "child" PRs. So, it should retain any secondary tags which were in the original PR, and the two level changelog should still work, although I have not tested this (yet).

fingolfin · 2025-09-12T10:43:21Z

Conflicts need to be resolved now.

And of course happy to discuss with @HereAround .

Note that this implements my proposal from #4673 and I hope people are already aware of that...

dev/releases/release_notes.py

lgoettgens · 2025-10-27T15:25:34Z

What is the state of this? It would be really great if we could have this in a few weeks before the next release, so that people can already learn about this and start using it (thus reducing the manual amount of work needed for the release).

aaruni96 · 2025-10-27T15:27:21Z

This is on my list right after the documenter business (which I am working on now). I hope to be able to get this done by next Wednesday (05 nov)

Previously , we did something really stupid and did half the work in two places.

aaruni96 · 2025-11-04T00:11:27Z

Now converts the body

---
## Release Notes
- Does abc {package: AbstractAlgebra, renaming}
- Does 123 {package: Nemo, documentation}
- Questions 42 {package: Singular, serialization}
---

into

### Changes related to the package AbstractAlgebra

#### Renamings

- [#5271](https://github.com/oscar-system/Oscar.jl/pull/5271) Does abc

### Changes related to the package Nemo

#### Improvements or additions to documentation

- [#5271](https://github.com/oscar-system/Oscar.jl/pull/5271) Does 123

### Changes related to the package Singular

#### Changes related to serializing data in the MRDI file format

- [#5271](https://github.com/oscar-system/Oscar.jl/pull/5271) Questions 42

This reverts commit e78c6e5.

aaruni96 · 2025-11-04T00:16:02Z

~~todo: documentation, adjust the "enforce labels" pr action to accept "use body" as a valid label.~~

lgoettgens · 2025-11-05T12:10:08Z

docs/src/DeveloperDocumentation/changelog.md


+#### Release Notes: Use Body
+
+It is possible to manually supply release notes in the body of the PR (the first


I believe the "body" is usually called "description"

The API calls it "body". I can change it to "description" if that is more widely understood as "first comment of the PR"

I am fine with using "body" as long as this term is used consistently. We can simply explain it better and even link it to the term "description". For example:

(the body of the PR is the first comment in the PR, created at the same time as the PR is created; it is also sometimes call the "description" of the PR).

lgoettgens · 2025-11-05T12:10:26Z

docs/src/DeveloperDocumentation/changelog.md

+#### Release Notes: Use Body
+
+It is possible to manually supply release notes in the body of the PR (the first
+comment in the PR). To do this, make a section in your PR title by putting a


This should be whatever is the first part called (body, or description, whatever the consensus is)

Wee it definitely shouldn't be "title", because that's simply wrong.

Suggested change

comment in the PR). To do this, make a section in your PR title by putting a

comment in the PR). To do this, make a section in your PR body by putting a

Yeah, calling it title was a mistake. But I don't want to change it before we have a decision for whether to call it 'body', or, whether to call it 'description'. (or, there is potential for a change, and then another change, if we pick the incorrect alternative the first time)

The label already uses body, the API uses body, and my code change suggestions here everywhere use body, so we're using body :-)

lgoettgens · 2025-11-05T12:11:12Z

docs/src/DeveloperDocumentation/changelog.md

+- item1 {label1, label2}
+- item2 {label3, label4, label5}
+- item3 {label5}
+


Is the empty line here required? If yes, please mention that. If not, can we remove it in all examples?

Strictly speaking, if you have 3 lines ( --- ) right after a line, that is, no empty line, then it is treated as a heading in markdown. It doesn't make a difference in our script, but not leaving that last empty line makes the markdown syntax highlighting color the last line differently, possible confusing someone?

We could also simplify this by leaving out the --- bits completely. Just say:

it starts on a line that is exactly ## Release Notes (I'd be OK with ignoring trailing spaces as those sometimes sneak in and are "invisible"; and I'd also be OK with ignoring case)

then we expects lines starting with - and of the form - description {label1, label2}

we ignore any empty (except for whitespace) lines before, between or after

as soon as we hit a line that is neither empty nor starts with - we stop the scanning.

Then you can properly parse it but without regex, but please also with the current search hack, as follows

split the body into lines, rstrip each line (to remove trailing whitespace)

scan the array of line until you find one matching ## Release Notes (ignoring case)

then iterate over the following lines and..

skip empty ones

parse those starting with -

abort the search if you find anything else

That is easy to explain, robust (no weird matches in the middle somewhere) and should be reasonably to use.

lgoettgens · 2025-11-05T12:12:42Z

I don't understand the python code, so I just reviewed the docs change

aaruni96 · 2025-11-12T21:37:00Z

Changes from current state can be previewed at 06019bf

fingolfin · 2025-11-13T00:49:23Z

dev/releases/release_notes.py

-    title = pr["title"]
-    return f"- [#{k}](https://github.com/oscar-system/Oscar.jl/pull/{k}) {title}\n"
+    if has_label(pr, 'release notes: use body'):
+        mdstring = pr['body'].replace("- ", f"- [#{k}](https://github.com/oscar-system/Oscar.jl/pull/{k}) ")


Won't this also replace any - that occurs in the middle of a description? Might be better to use a regex anchor, i.e., something like

Suggested change

mdstring = pr['body'].replace("- ", f"- [#{k}](https://github.com/oscar-system/Oscar.jl/pull/{k}) ")

mdstring = pr['body'].replace(r"^- ", f"- [#{k}](https://github.com/oscar-system/Oscar.jl/pull/{k}) ")

I don't know why, but as of this moment, I am unwilling to use regx. I have tried to make an alternate solution: replace only the first occurrence in string.

I think regx (dash only at first position) might be a stronger solution, but my most recent changes might be enough in this case.

I will change it to regx if you think its better in any case.

Well, there is a concrete bug that can relatively easily occur with your current code, so yeah, I really think you should either use the regex here, or use some other approach that ensures the dash is at the start of a line (e.g. you could also match for "\n- " and replace by "\n- ...")

fingolfin · 2025-11-13T00:52:18Z

dev/releases/release_notes.py

+        return body
+    index2 = body.find('---', index1)
+    # there are 17 characters from index 1 until the next line
+    mdstring = body[index1+17:index2]


But what if there is a line ## release notes follow below instead? Or if the trailing --- is missing? All of these can easily happen by accident.

How about instead using a regex like this (untested, just meant as a rough guide), and then extracting the correcting matching group. (Of course multiline regex matching must be enabled for this, however one does that in Python)

r"^## release notes\n(.+)^---\n"

For the same strange reason as above ("gut feeling"), I tried to solve this problem without using regx.

I will change it to the regx solution if you think it is a better solution than my latest commits

dev/releases/release_notes.py

lgoettgens · 2025-11-13T09:47:02Z

@aaruni96 I made another comment regarding this at #5544 (comment)

fingolfin · 2025-11-13T14:22:03Z

dev/releases/release_notes.py

 # the given label is put into the corresponding section; each PR is put into only one section, the first one
 # one from this list it fits in.
 # See also <https://github.com/gap-system/gap/issues/4257>.
-topics = [


There are a bunch of changes in this PR that are independent from the actual purpose of this PR, but each time I look at the python code, I have to filter them out mentally "ah yes I already looked at this, it does XYZ and is fine".

It would really help if that would go into another PR, or at least a separate commit, that only does that refactoring but nothing else.

Specifically

changing topics and prtypes into dicts

renaming startdate to timestamp

dropping an unused startdate argument

removing a print statement

I think that would cut down the diff in half and could be merged right away.
Plus we'd be free to work on renaming the labels as @joschmitt suggested elsewhere in a separate PR (as this PR here would no longer touch the list of topics/prtypes)

Sorry, this PR got out of hand.

Would you prefer :

Splitting out this into 2 PRs, one which just deals with the multiple entries for a single PR, and another which deals with other things

or

Rewriting some history, to merge all the "patchwork" commits into one commit, and leave the commits which actually tackle the topic at hand separate (I think this might be more involved?)

I think 1 would be easier

fingolfin · 2025-11-13T14:28:17Z

docs/src/DeveloperDocumentation/changelog.md

+in the changelog for a single PR. It is possible to label each of the entries
+with their own topic / pr type labels.
+
+The syntax is the following :


Suggested change

The syntax is the following :

The syntax is the following:

aaruni96 added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Sep 8, 2025

aaruni96 force-pushed the ak96/changelogscript-multiple-entries-single-pr branch from 0d7ed72 to c40d3ae Compare September 8, 2025 20:56

aaruni96 requested a review from fingolfin September 8, 2025 21:39

aaruni96 added 4 commits September 17, 2025 11:23

Step1: grab release notes from body

e914249

Turn topics and prtypes from list to dict

a89ca7d

Hacky solution to add topic labels

8806969

bugfixes after rebase

c609105

aaruni96 force-pushed the ak96/changelogscript-multiple-entries-single-pr branch from c40d3ae to c609105 Compare September 17, 2025 09:33

fingolfin reviewed Sep 17, 2025

View reviewed changes

dev/releases/release_notes.py Outdated Show resolved Hide resolved

dev/releases/release_notes.py Outdated Show resolved Hide resolved

dev/releases/release_notes.py Outdated Show resolved Hide resolved

dev/releases/release_notes.py Outdated Show resolved Hide resolved

aaruni96 added 3 commits November 4, 2025 00:46

edits for having lots of labels

90cfd0d

to revert later

e78c6e5

Address review comments

cfe0d85

Previously , we did something really stupid and did half the work in two places.

Revert "to revert later"

87a4b3e

This reverts commit e78c6e5.

aaruni96 added 4 commits November 4, 2025 01:19

merge the obviously merge-able if clauses

becb7ea

Handle some problems by complaining and going through anyway

d17b11a

Adjust the CI action

9cc0082

Update docs

263440c

aaruni96 requested review from fingolfin and lgoettgens November 4, 2025 15:06

lgoettgens reviewed Nov 5, 2025

View reviewed changes

fingolfin mentioned this pull request Nov 12, 2025

Release OSCAR 1.6.0 in November? #5536

Open

aaruni96 added 3 commits November 12, 2025 22:14

use full timestamp for PRs

fc2151b

don't edit the same list we are currently iterating

494e65d

remove accidental garbage from printing

b9088d8

aaruni96 mentioned this pull request Nov 12, 2025

Update changelog #5544

Draft

fingolfin reviewed Nov 13, 2025

View reviewed changes

dev/releases/release_notes.py Outdated Show resolved Hide resolved

aaruni96 added 4 commits November 13, 2025 09:13

don't bother removing things from loop we don't use again

935c31e

Replace only the first occurance of dash

5cca31e

better capture of the actual release notes section

50f238c

remove unused parameter from function

2a6a6d6

fingolfin reviewed Nov 13, 2025

View reviewed changes

lgoettgens mentioned this pull request Nov 14, 2025

Isometry testing and groups via decomposition #5545

Merged


		#### Release Notes: Use Body

		It is possible to manually supply release notes in the body of the PR (the first

	comment in the PR). To do this, make a section in your PR title by putting a
	comment in the PR). To do this, make a section in your PR body by putting a

	mdstring = pr['body'].replace("- ", f"- [#{k}](https://github.com/oscar-system/Oscar.jl/pull/{k}) ")
	mdstring = pr['body'].replace(r"^- ", f"- [#{k}](https://github.com/oscar-system/Oscar.jl/pull/{k}) ")

Changelog script: multiple changelog entries for a single PR #5279

Are you sure you want to change the base?

Changelog script: multiple changelog entries for a single PR #5279

Uh oh!

Conversation

aaruni96 commented Sep 8, 2025 • edited by fingolfin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fingolfin commented Sep 10, 2025

Uh oh!

lgoettgens commented Sep 11, 2025

Uh oh!

aaruni96 commented Sep 12, 2025

Uh oh!

fingolfin commented Sep 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgoettgens commented Oct 27, 2025

Uh oh!

aaruni96 commented Oct 27, 2025

Uh oh!

aaruni96 commented Nov 4, 2025

Uh oh!

aaruni96 commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgoettgens commented Nov 5, 2025

Uh oh!

aaruni96 commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lgoettgens commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

aaruni96 commented Sep 8, 2025 •

edited by fingolfin

Loading

aaruni96 commented Nov 4, 2025 •

edited

Loading