esp-rs
Proposal to improve changelog handling
Currently, the keeping changelogs causes some minor, but nontrivial friction:
- Even trivial changes require a changelog entry, unless maintainers tell otherwise with a label
- Keeping the changelog requires knowing the PR number up front
- The responsibility to add entries is on the contributor
- Multiple PRs can end up conflicting on the changelog only, which reduces the efficiency of the merge queue if a PR gets rejected partway through the process
My proposal is to stop tracking the changelog and the migration guide additions in tree. Instead, we should add some markup to the PR description, where we, the maintainers (or the more advanced contributors, but only optionally) can propose the changelog entry/migration guide addition.
The xtask can, in the release planning phase, collect these PR descriptions, extract the data and modify the changelogs/migration guides.
The exact syntax of the PR description is subject to bikeshedding. The format should be rich enough to be unambiguous, contain which crate(s) are changed and how, and optionally which area in a crate has been changed (for example, a peripheral like SPI/RMT could be one such area). If we get the format right, the PR descriptions may be informative enough without much additional comment. As an initial idea, I propose we just use markdown syntax with a slightly restricted format:
## Change: esp-hal/LOL-area
- Added: FOO
- Changed: BAR
- Removed: BAZ
## Change: esp-radio
- Changed: Public API changes related to LOL-area
## Migration: esp-hal/LOL-area
Lorem ipsum with diff
## Migration: esp-radio
Lorem ipsum
I propose we make it clear that keeping the changelog is not the responsibility of the contributor, but we appreciate them keeping to this format. We can easily edit the PR descriptions ourselves, instead of going back-and-forth with contributors on typos/stylistic issues.
To implement this idea, we need:
- First, discuss the approach, and come up with a finalized design.
- A CI workflow that checks for changelog entries in the PR description only if the PR description contains changelog-looking headings. Changelogs are optional, but their format is a bit rigid, so we need to make sure the tooling can cope.
- A CI check that turns direct changelog edits into errors, and comments on the PR how we expect (or expect not to) keep the changelog.
- The xtask needs additional code to collect the changelog entries and turn them into documentation.
- The release process needs to change to allow for manual fine-tuning of the changelog documentation.
- The release process needs to either be able to retrieve the PR description at the time of the merge, or allow for misformatted PR descriptions, as they can be edited at any time.
The advantages are:
- No more merge conflicts
- No more wasted time on updating the PR number in the changelog after the PR has been opened
- Contributors will no longer try to reference issue numbers in the changelog
- We can create a changelog that is grouped by the area, i.e. we will group SPI-related changes together by default
- We can edit the changelog ourselves, which means the end result should be more cohesive.
I'm looking forward for thoughts, opinions, general feedback and everything else that is constructive on this matter.
Interesting idea
Just for my understanding: we will keep the CHANGELOG.md etc. files but the xtask will "maintain" them? Or do you plan to store them somewhere else?
I did not think about that, I assumed we would keep the markdowns. However, since we're posting the notes on github releases as well, we can probably remove these from the tree completely. I'm undecided, and I think this question is an (important) implementation detail that we can make a decision about at any time.
To me anything that resolves
- merge conflicts
- wasted time on updating the PR number in the changelog after the PR has been opened
sounds like a great thing to have!
One thing that came to my mind is, that we also need to apply that for backport branches / releases. Not sure if it will be a no-brainer and needs special consideration at all. (with the current process we get them "for free": https://github.com/esp-rs/esp-hal/blob/esp-hal-1.0.x/esp-hal/CHANGELOG.md )
One potential drawback might be that we don't have a very easy way to see what is already there for the upcoming release (i.e. we and the community can just have a look at CHANGELOG.md - for sure solvable ... just mentioning)
One potential drawback might be that we don't have a very easy way to see what is already there for the upcoming release (i.e. we and the community can just have a look at CHANGELOG.md - for sure solvable ... just mentioning)
Yes, this would be a minor thing we lose.
Maybe we could have a ci action (using and xtask command) that populates the changelog.md file from the PR description when the PR is merged, this should avoid the git conflicts (?)
Maybe we could have a ci action (using and xtask command) that populates the changelog.md file from the PR description when the PR is merged, this should avoid the git conflicts (?)
This has been proposed for embassy, and it is thought to break the merge queue. The github action could maintain a github issue with the current draft changelog, or we could provide an xtask command to print it, but the whole idea is based on NOT tracking the in-progress changelog in-tree.
I'm not bothered whether we track the changes in tree or not, but I'm concerned about things getting missed or not making their way into the final release notes. So provided we can implement the checks you're suggesting to ensure that relevant PRs have changelogs/migrations I'm on board :D.
I have a few questions from your RFC:
The xtask can, in the release planning phase, collect these PR descriptions, extract the data and modify the changelogs/migration guides.
I guess we need to figure this bit out. One suggestion could be to open a draft release for the next release, and append these bits there over time. I think this could be automated with gh API / some kind of action. This would allow us (maintainers) to see pending changes, but also to refine them in the place where they'll be at the point of release (edits, fixups etc).
A CI workflow that checks for changelog entries in the PR description only if the PR description contains changelog-looking headings. Changelogs are optional, but their format is a bit rigid, so we need to make sure the tooling can cope.
Should we consider a human-ish read/writable format, like yaml for structuring these logs? I think trying to parse free form text from various contributors (including ourselves) would be painful. I'm not sure what to do about migration guide entries though, as those often rely on markdown - maybe we can just copy these verbatim?
No more wasted time on updating the PR number in the changelog after the PR has been opened
I think there is still value in having the PR number associated with the changelog, could we have the tooling inject this?
I think there is still value in having the PR number associated with the changelog, could we have the tooling inject this?
That's the idea, yes.
One suggestion could be to open a draft release for the next release, and append these bits there over time.
The process itself is easy - check out main, run xtask, figure out the draft release PR, and force push to its branch. However, by default that generates a notification, and triggers a CI run. Updating the description of an issue involves about the same steps, but it is much less annoying.
but also to refine them in the place
This part assumes the manual changes are kept, which would just result in a lot of edge cases. Which changelog entries need to be considered? How will the tool know which one was edited, and which one is new? How will the process handle merge conflicts between the old draft release and the current main? I think a single pass edit at the end of the release cycle is less error-prone.
to ensure that relevant PRs have changelogs/migrations
That will be the responsibility of the reviewer and the PR author together (because we don't forbid the contributors from adding the documentation).
That will be the responsibility of the reviewer and the PR author together (because we don't forbid the contributors from adding the documentation).
Its fine that its both or either's responsibility to actually write the entry but I'm concerned that it will get missed completely without some automated check in place like we have currently. I'd like the skip-changelog label workflow to still be possible with this approach, where not writing a changelog entry is opt-out instead of the default.
Maybe we can just wait a bit and see how "that other project" likes their approach and learn from them 😄
Which other project?
If I understood you correctly embassy is going into a similar direction (but I might got you wrong and I admit I didn't check that) - if not we shouldn't wait for them ofc
embassy turned off their changelog check completely, and while there are ideas to ease writing the changelog before release, nothing concrete has been done there.
embassy turned off their changelog check completely, and while there are ideas to ease writing the changelog before release, nothing concrete has been done there.
Thanks for the update - ok that's interesting