Require Changelog Entries for each PR

[pascalkuthe]

This PR is the result of previous discussion in the matrix chat.
@archseer noted that writing up change-logs is extremely tedious/time intensive but that none of the existing automation tools satisfy his requirements.
Furthermore a changelog for the current unreleased changes in master was requested.

This PR implements an alternative solution to reduce the load on the maintainers.
I have implemented a simple CI workflow that ensures that every PR provides a changelog entry.
This ensures that a changelog is always maintained in master while reducing the burden on the maintainers.

To ensure this process can work henceforth I have written up a changelog since the last release.
One caveat with this approach is that already open unmerged PRs use the CI from their own branches and won't be automatically updated so there will be a bit of a transitioning period. If this PR is merged I will try to suggest changelog entries for PRs (tagged waiting for review) to smooth that transition.

[poliorcetics]

every PR provides a changelog entry.

I don't think that's important enough to require it for every PR. Lots of first-contributor PRs are "fixed a typo", "fixed a color in this theme", "optimized just this function", requiring them to write a changelog entry for such small internal changes is one quick way to lose them

[poliorcetics]

What we could do instead is add it to the template discussed in #4002, with instructions on when it is required. That way maintainers can force push a changelog commit if the PR is simple but deserve one and it's not there and repeat contributors will know about it already and do it themselves

[pascalkuthe]

If you look at the existing changelog even small changes like typos (inside helix like the tutor) get a changelog entry.
Adding a changelog entry is not difficult and I think its honestly reasonable to expect a contributed to write a small single sentence to describe their changes (they already have to do that for the PR to some extent anyway).

The benefits of this approach is precisely that changelogs are written by contributors and not maintainers to free up their time.
The second reason why I think this approach is preferable is that we check the changelog in every PR so contributed and maintainers don't forget about it.

However we could combine this approach with a PR template with a checkbox for a changelog entry to make the requirement more discoverable.

Where I do think it makes sense to remove the requirement for changelog entries is updates to the documentation (and changes by depandabot). These are the only changes where no changelog entry is created. I think that can easily be achieved with a path filter for the CI job

[archseer]

I don't like requiring this because it adds an additional hurdle to the contribution process. PRs are now held up by missing changelog entries and I think it'll lead to more bikeshedding on what the entry should say (some git users already don't write proper git commit messages in the first place). Not to mention potential merge conflicts when PRs append to the same changelog file.

It's tedious to do this afterwards but maybe we could at least have a generator that would do the basic entries which we could then further edit.

[pascalkuthe]

I can see that being a problem. I will look into setting up a generator but I am not sure there is a good solution for that.

I was very rarely able to reuse commit messages and often had to dig trough both the PR and related issue to figure out what a change actually does and find a message that actually means something to a user (you are probably faster because you actually reviewed those changes before). Furthermore the chosen changelog format relies a lot on semantic information. For example "add darker variant of onedark" goes into the theme category as something like add onedarker. From the commit message there is no way to tell that this is a theme. The only way I can see putting the changes into the right category would be to use a generator that uses github PR labels. But the messages would probably not be that great (bestcase I can see is to reuse PR titels) and require editing.

[pascalkuthe]

The existing solutions for something like this kind of don't do what we need for helix. I wanted to write something like that for a while, should not be too hard, I will look into that.

[dead10ck]

I agree with @pascalkuthe that a change log entry isn't that much to ask, but I do see why we might not want to make it a hard requirement. Is there a softer way we could do this? Maybe just have the CI set a warning?

[A-Walrus]

Quick note, this PR (#3425) should not be in the languages category it should be in Usability improvement and fixes. The markdown it's referring to is the markdown in LSP help popups.
(By having the PR authors write the CHANGELOG like you said this kind of stuff could be avoided :P)

[poliorcetics]

Maybe just have the CI set a warning?

Yes, an step in allow-failure sounds nice, that way newcomers are warned that something is still to do but it doesn't immediately says "FAIL" in their face, which I find very discouraging. That plus a PR template looks like a sweet spot.

[the-mikedavis]

I don't like requiring contributors to write their own entries: not only is it an extra hurdle for contributions and review as archseer says but you end up with different writing/punctuation styles and focuses in the changelog. I'm also not a fan of auto-generating them: changes frequently need some copy-editing of the given commit/PR messages/titles. Some changes need to be merged into the same line (for example repeated updates to one languages' queries, or fixes for a newly merged feature) or ommitted (tutor typo fixes or small internal refactors - I intentionally omit several changes per release).

I prefer to write the changelog myself. I try to work through the changes weekly and push a changelog branch but I'm behind on that for this next release 😅

[dead10ck]

Yeah, this all makes sense. Would you be against it if contributors don't mind writing their own entries? I understand differences in style, etc, but this could be part of review as well right? It might make it easier to take a minute for each PR a dozen times over all PRs instead of dozens of minutes at once.

I think we're all just looking for ways to help spread the load, but at the end of the day, it's you guys who end up doing the work (most of the time - thanks @pascalkuthe!), so if you really prefer just writing them all by hand, then yeah I guess there's no need for this change.

[poliorcetics]

Even if we currently have @the-mikedavis doing it, this is a bus factor of 1. If you're sick, absent, not interested anymore, it becomes a limitation for Helix (not by your fault, all cases I cited are completely valid).

We may not require it, but motivating people to write a changelog entry if possible reduces this bus factor (and helps spread the load too, as @dead10ck said, which is a big plus)

[pascalkuthe]

I think the points raised by @the-mikedavis and @archseer make a ton of sense.
Especially wrt to merge conflicts and bikeshedding and PRs.
Like @the-mikedavis I also merged (and omitted) changes while writing the PRs by hand.
I have come up with a scheme to automate as much as possible while still obtaining the handwritten approach.
I already have a prototype tool for automating this locally but its not quite ready yet:

The idea of this tool is that changelogs are kept in a machine readable and writable format until the release.
I choose toml for now but with serde (tool is written in rust) this can easily be changed.
The tool has the ability to automatically render the changelog into something nice (using tera templates; I already have a config for the tool that replicates the current format).

While this format is machine readable and writable it is also editable by hand.
An example of what the format currently looks like (very early development, definitely not final, contents are just a mock-up do not take this seriously):

authors = ["pascalkuthe", "archseer"]

[[changes]]
message = "Fix crash XYZ"
group = "Usability improvements and fixes"
pr = [1457]

[[changes]]
message = "Change keybinding XYX"
group = "Breaking changes"
pr = 3300

[[changes]]
message = "Add Ability to Display diffs and act as a difftool"
group = "Features"
pr = [3415, 2789]

The tool can already render this toml into the following changelog (with a template that matches the current format)

22.10.1

A big thank you to our contributors! This release had 2 contributors.

As usual, the following is a summary of each of the changes since the last release.
For the full log, check out the git log.

Breaking changes:

• Change keybinding XYX (#3300)

Features:

• Add Ability to Display diffs and act as a difftool (#3415, #2789)

Usability improvements and fixes:

• Fix crash XYZ (#1457)

This changelog file is maintained in a seperate branch (and never merged into master).
Whenever a PR is merged the tool runs in CI.
There it reads this toml changelog, adds an entry for the PR to the toml file and commits that toml file to a branch.

Because the tool only adds to the toml file while leaving existing content untouched, maintainers can easily edit the changelog whenever they feel like working on it by simply pushing to the branch.

The tool also runs in the CI for this changelog branch.
Whenever this branch is pushed to (either caused by a merge into master or a manual edit of the changelog branch) it renders the json file into a human readable changelog and update (or creates) a draft PR into master.
This PR is not intended to ever be actually merged and instead just servers to display the current unmerged change (even if they are not perfectly formatted yet such an overview is quite useful).

Whenever a release is ready the toml is edited by the maintainers until they are happy and rendered with the tool locally. The output can then simply be pasted/piped into CHANGELOG.md.

The default change for a PR is generated as follows:
By default the title of the PR is used as message.
Contributors can provide a changelog: <MESSAGE> line in the body of their PR that is used instead of the title.
The default group can be set in the config file.
Additionally the config file allows mapping PR labels to certain groups (for example A-theme to the Themes category). For this to actually be effective for helix a couple more PR labels would be required.
However if the maintainers prefer editing by hand later then these rules can just not be added.

This allows maintainers to do everything they already do by hand (deleting changelog entries, merging multiple entries, nice text), keeps changelogs out of normal PRs (avoid merge conflicts and bikescheeding) while still making change-log generation a lot less manual/cumbersome.
This approach also allows contributers to helpout by specifying their own changelog message.
This message is just a default to make the maintainers life easier that can/will be edited by maintainers before it shows up so the concerns.

While this sounds quite complicated its actually not that hard to implement (I already have most of this working) and pretty easy to use in practice: Contributors won't have to do anything (but may optionally add a changelog line to their pr description). Maintainers get a pretty decent starting point for their changelog which they can still edit however they want and can do their changelog in batches (I could convert my changelog here to toml for example and the tool would just keep adding new changes on top).

What do you think?

[the-mikedavis]

It seems like too much solutioning for a problem that doesn't really need it. I would prefer to leave the changelog process as-is.

this is a bus factor of 1

I suppose, but the process is documented and is pretty straightforward besides; anyone could take over in a pinch.

Would you be against it if contributors don't mind writing their own entries?

I wouldn't really mind it but making changes directly to the changelog file in a PR makes for a lot of conflicts. I would prefer that contributors refine / edit the PR title since I tend to take that outright most of the time