RFC: CHANGELOG overhaul

[Conaclos]

Hi!

I would like to change the way we write the CHANGELOG. I pretty like how the author of esbuild write its CHANGELOG. However, I accept that it is certainly too much work. We could target a more achievable goal. I am thinking to a CHANGELOG that looks like what I am doing for my personal projects.

Some months ago, I started a guide for writing CHANGELOG. I think we can take some inspiration of it. The key tip is A changelog entry should add context and motivation.

I think we should update at least the last release CHANGELOG to give to devs a concrete example of what we think is a good CHANGELOG entry.

[strager]

I would like to change the way we write the CHANGELOG.

What are your problems with Rome's current change log?

[Conaclos]

From a user pov, It is a bit cryptic and often require diving into issues to understand what the change is doing.

[ematipico]

Could you provide some concrete examples of how you would write changelog entries that would fit Rome's structure?

Rome has different areas (CLI, LSP, formatter, etc.) as you know. I'm very excited to get our changelog to the next level :)

[Conaclos]

What I'm seeing is that in esbuild the contributors don't write the documentation, but it's Evan that does that at the very end (I assume).

In fact, Ecan is mostly the only contributor to esbuild. Thus, he is, indeed, the only changelog writer.

I accept that this is a burden to write a changelog entry.
However, it also helps in writing a good commit message.
Unfortunately, rome favors extremely short commit messages.

It is also a burden to write the entire changelog before a release (what you was doing before requiring contributors to add a changelog entry?).

We could adopt a hybrid approach:

• Keep the current recommendation to write a changelog entry and then suggest to the contributors how to improve it
• Improve the changelog before a release, and on a regular basis (fixes, ...).

Not sure which way to go :/

[Conaclos]

I believe that Evan uses the changelog as a "blog post" for a release, I might be wrong.

Maybe.
I think we should not write so detailed changelog entries.
Just adding what the change is and some context and/or motivation should be enough.

[ematipico]

If we decided to remove the burden for contributors to write the changelog line, how do we (maintainers) keep track of the changes we want to track?

Do you have a proposal? In the past I used to go through all the merged PRs and skim them, but that used to take a lot of time. I was thinking about using some labels e.g. "To document" or "To track in the changelog", or something like that.

Any other ideas?

[Conaclos]

To reduce the burden of this task, I think the changelog entry should be written as soon as possible.
This could be done by:

• releasing frequently
• pushing a commit that edits the changelog on every PR before merging

Another suggestion is to make changes to document for a given release easily discoverable.
This could be achieved by adding:

• a milestone
• tag (as you suggested)

I don't know if we can automate some of this?

[strager]

If we decided to remove the burden for contributors to write the changelog line, how do we (maintainers) keep track of the changes we want to track?

In my main open source project (where I am the sole maintainer), I do this:

• When I write my own patches, I remember maybe 50% of the time to write a change log entry.
• When I accept contributions, I never ask them to write a changelog entry.
• Every once in a while, including before a release, I look through all patches since my last chore(docs): update change log commit. (Something like git log --oneline is usually enough.) I make additions to the change log based on these patches.
• A commit message convention such as Conventional Commits helps ignore refactor commits.

Benefits of this approach:

• I only write change log entries when I'm in the mood.
• Contributors don't need to worry about the change log. I worry less about a contributor's English skills.
• I have fewer Git merge conflicts in the change log file.
• The change log is written in a [mostly] consistent voice.

Downsides of this approach:

• Might not work for someone who hates writing technical documentation. (But I don't think anything will...)
• Looking through the commit log might be a pain if people did a bad job explaining the patch in the commit message. It may have been a month since I looked at the patch, so I may not remember the context of the patch. I'd need to spend time re-remembering.
• It might work for one maintainer who sees all patches, but maybe not with multiple maintainers who work in different parts of the code.

[ematipico]

Do you want to keep the existing manual process, or do you want to propose to automate the update process of the changelog?

[Conaclos]

What do you mean by automatic update? Automatic generation? I think that it is not a good idea. Generated changelog are often poorly written...

[ematipico]

Yeah, I agree with you. An automated process wouldn't work.

[strager]

I reviewed the change log entries for the last few minor releases (12.1.1 through 12.1.3). My thoughts:

• Entries should link to documentation if possible. For example, "Accept the environment variable 'ROME_BINARY' to override the Rome binary" doesn't explain why I would use ROME_BINARY or how to use it exactly. I can't find documentation for ROME_BINARY, so maybe it's not even documented. EDIT: Oh, docs are here: docs: Add documentation for ROME_BINARY #4582
• "Fix an issue where all the nursery were enabled when the "nursery": {} object was defined" is a breaking change (I think) but not was not labelled as one.
• "Revert feat(rome_js_parser, rome_js_formatter): import attribute #4359" is completely unhelpful. It should instead read as two entries:
  • "Reintroduce support for import "module" assert {} syntax. Support was dropped in version X.Y by mistake."
  • "BREAKING CHANGE: Drop support for import "module" with {} syntax. This feature was enabled in the version X.Y by mistake."
• "Revert feat(rome_js_parser, rome_js_formatter): import attribute #4359" is mentioned twice for some reason.
• "noInnerDeclarations: allow function declarations in nested block inside an ES module" had a broken link. I fixed it in docs: fix broken link in change log #4588.
• "noInnerDeclarations: allow function declarations in nested block inside an ES module" links to a task. That task links to the Rome Playground, but the link isn't linkified. If I click the link, I can see what the issue is about. Perhaps the example should be present in the change log directly? Then when I read the change log description, I'm confused. I think the wording is wrong; perhaps it meant to say "disallow"? Also, in my mind, functions count as nested blocks, so the wording makes it sound like "allow function declarations in functions inside an ES module", which is unrelated to the lint rule... Or maybe I'm just not understanding anything correctly at all. I guess that's my point: the change log entry is unclear!
• "noInvalidConstructorSuper: recognize extends clauses that use static member access such as extends mod.C" is a good description, except I don't see what it has to do with noInvalidConstructorSuper. Is it saying that noInvalidConstructorSuper was not triggered if the class was declared as class C extends react.Component { constructor() {} }, but now it is triggered? Maybe this is just my ignorance of the noInvalidConstructorSuper lint rule.
• "Fix regression where the command lsp-proxy was renamed lsp_proxy" is good, but it should mention which version it broke in.
• In version 12.1.1's docs, the wording is "Fix an issue where the noAssignInExpressions rule ...". In version 12.1.2's docs, the wording is "noInvalidConstructorSuper: recognize ...". I like 2.1.1's style better personally, though it is a bit wordy.
• "Fix an issue where the noAssignInExpressions rule replaced the operator with an invalid token, which caused other lint rules to crash." I like this because it explains that a crash was fixed.
• "Fix an issue that noUnusedVariables rule did not correctly detect exports when a variable and an interface had the same name" is clearly written. 👍
• Some entries linkify the lint rule names and some entries don't. They should all linkify. (EDIT: I fixed some instances in docs: linkify lint rules in change log #4589.)

Notes about process:

• While I was creating docs: fix broken link in change log #4588, I noticed "[ ] The PR requires a changelog line". In my opinion, it is the job of maintainers to maintain the change log. Drive-by contributors should not be pressured to think about the change log at all. (They can add things if they want, of course.) I would remove this checkbox from the PR template to reduce contributor fatigue and to reduce the chance that maintainers use an excuse like "they didn't say it was important to put in the change log".

[Conaclos]

This is a really good review! Feel free to submit a PR to improve the changelog if you have time to do it :)

In my opinion, it is the job of maintainers to maintain the change log. Drive-by contributors should not be pressured to think about the change log at all. (They can add things if they want, of course.) I would remove this checkbox from the PR template to reduce contributor fatigue and to reduce the chance that maintainers use an excuse like "they didn't say it was important to put in the change log".

This is similar to the remark of @ematipico.

[ematipico]

Thank you @strager for your great feedback 🙏 ❤️

I totally agree with you that it should be the job of a maintainer to write a good changelog. The main reason why I took this approach is mostly because it took a lot of energy to keep up with changes, mainly tracking them.

Before it was my full-time job, so it wasn't a big deal. I noticed that jest uses this approach - let the contributor write the changelog line - so I went with it.

I am going to review how we could better track changes without burdening contributions with this task.

[nissy-dev]

It is great to improve CHANGELOG.md 👍 I also shares my opinion.

From a user pov, It is a bit cryptic and often require diving into issues to understand what the change is doing.

I understand this perspective, but I think users sometimes have to dive into issues.

It is the job of maintainers to maintain the change log. Drive-by contributors should not be pressured to think about the change log at all.

How should we do it then? Reviewing and making a good changelog is a big effort that not all contributors can afford to do (maybe some of them don't speak English very well).

I have similar opinion. As English is not my native language, it takes me a little time to write changelogs. I think that it is better to generate CHANGELOG.md from commits automatically, and maintainers should explain only important changes in the details.

[ematipico]

@Conaclos considering the discussion, would you like to propose some changes to our contribution guidelines? For both contributors and maintainers?

[Conaclos]

I will :)