Skip to content
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

Automating changelog maintenance #1182

Open
rmacklin opened this issue Dec 4, 2021 · 17 comments
Open

Automating changelog maintenance #1182

rmacklin opened this issue Dec 4, 2021 · 17 comments
Labels

Comments

@rmacklin
Copy link
Collaborator

rmacklin commented Dec 4, 2021

After #1181, I'd like to suggest automating the maintenance of ViewComponent's changelog.

While I love a good hand-crafted changelog, I think the conventions already in place in this repository make it worthwhile to pursue changelog automation:

Those conventions make git log a good fit for changelog generation.

Automating the changelog management would solve a few issues:

  • CHANGELOG.md is a frequent source of merge conflicts among the PRs that aren't merged in a short window.
  • It's possible to miss an entry in the GitHub Release description because a change was listed under the wrong version when merging CHANGELOG.md. (Move changelog entry for #1162 to the right version #1181)
  • We have /CHANGELOG.md as a symlink to /docs/CHANGELOG.md, which doesn't work for some tools (CHANGELOG.md looks like an empty file without any links #1170).
    • We can address this by generating the changelog content in both locations (which would remain in sync since the updates would be totally automated).
  • Maintainers who modify a PR's changelog entry before merging could be more efficient (Compiler redefines .call in thread-safe way #1161 (review)). Instead of adding a suggestion and committing it (two steps, with an extra CI run), maintainers can just edit the PR title or the squash commit message.
  • Currently, the changelog doesn't link to PRs. PRs often have additional context, though, so it'd be nice to make them easier to access directly from the changelog entries.
    • When squash-merging, the PR number is automatically appended to the generated commit's subject, so generating the changelog from git log would have that covered.

Thoughts?

@joelhawksley
Copy link
Member

@rmacklin this generally sounds good to me. How do you suggest automating the changelog? For what it's worth, we've basically just copied what Rails does, which is manually writing entries 🤷🏻

@boardfish
Copy link
Collaborator

Seems to me like the solution would be a GitHub Action - it'd takes the contents of a squash commit after a merge and append them to the changelog under the main header. I guess the process of then moving what's under main to a release's header could remain manual, since the main pain point we're aiming to solve here seems to be having to manually resolve the conflicts that arise when you rebase.

@rmacklin
Copy link
Collaborator Author

rmacklin commented Dec 8, 2021

For what it's worth, we've basically just copied what Rails does, which is manually writing entries

I think the main difference with Rails is the first bullet - Rails doesn't require all changes to have a changelog entry (in fact it's explicitly discouraged for "refactorings and documentation changes" to have changelog entries)

I suppose it's open for discussion if we want to continue the practice of requiring every single PR to have a changelog entry. As noted in the issue, that practice combined with the practice of squash-merging all PRs effectively makes it such that the changelog is almost equivalent to the repository's commit log. Given that perceived redundancy, should we consider treating the changelog differently, more akin to how Rails does? From the perspective of consumers of ViewComponent who are upgrading to a new version, the current process could be seen as suboptimal, where the changes that don't affect the runtime are adding "noise" to the changelog. And if the changelog scope was reduced, the repository's commit log and contributor data would still provide recognition for each contribution. So, I think it's a valid question...

But I opened this issue assuming we would keep the current practices. If we do, I was thinking of extending the release script to handle inserting the new changes for each new release. Unreleased changes wouldn't appear in the changelog, though we could include a link to those at the top of the changelog. I can throw something together to illustrate what I'm envisioning, but definitely let me know if you're considering changing the scope of the changelog, as that would make it less straightforward to implement (though still doable, I think).

@rmacklin
Copy link
Collaborator Author

rmacklin commented Dec 8, 2021

Seems to me like the solution would be a GitHub Action - it'd takes the contents of a squash commit after a merge and append them to the changelog under the main header.

I considered a GitHub Action, too, but I think handling it in the release script might be simpler and cleaner - particularly since the release script is already modifying the changelog.

@boardfish
Copy link
Collaborator

I found github-changelog-generator on my travels. That's one potential option.

It looks as though it's potentially configurable to the point of being identical to what we have now. I'll have a play with it and see what I can do.

@boardfish
Copy link
Collaborator

boardfish commented Jan 4, 2022

I got pretty decent results by calling it as follows:

github_changelog_generator -u github -p view_component --simple-list --no-issues --pr-label="" --usernames-as-github-logins
It comes back looking a little something like this:
# Changelog

## [Unreleased](https://github.com/github/view_component/tree/HEAD)

[Full Changelog](https://github.com/github/view_component/compare/v2.47.0...HEAD)

- Fix style in generator docs [\#1228](https://github.com/github/view_component/pull/1228) (@Spone)
- Fix ruby 3.0 CI [\#1224](https://github.com/github/view_component/pull/1224) (@Spone)
- Add ViewComponent::Collection to @param of \#render\_inline [\#1215](https://github.com/github/view_component/pull/1215) (@yykamei)
- Make ViewComponent::Collection a collection [\#1178](https://github.com/github/view_component/pull/1178) (@sammyhenningsson)

## [v2.47.0](https://github.com/github/view_component/tree/v2.47.0) (2021-12-20)

[Full Changelog](https://github.com/github/view_component/compare/v2.46.0...v2.47.0)

- release 2.47.0 [\#1214](https://github.com/github/view_component/pull/1214) (@joelhawksley)
- Add the WEBrick gem to the docs app [\#1211](https://github.com/github/view_component/pull/1211) (@cpjmcquillan)
- Update `.tool-versions` to latest Ruby version [\#1209](https://github.com/github/view_component/pull/1209) (@cpjmcquillan)
- Add test for a lambda slot with a block and no other arguments [\#1208](https://github.com/github/view_component/pull/1208) (@boardfish)
- Add test for trailing newline with `render_in` [\#1207](https://github.com/github/view_component/pull/1207) (@boardfish)
- Add file consistency linters [\#1206](https://github.com/github/view_component/pull/1206) (@boardfish)
- Add @boardfish to docs index [\#1201](https://github.com/github/view_component/pull/1201) (@boardfish)
- Set up Codespaces for bug replication [\#1200](https://github.com/github/view_component/pull/1200) (@boardfish)
- Make @boardfish a committer [\#1191](https://github.com/github/view_component/pull/1191) (@joelhawksley)
- Ensure component delegation slots can use helpers [\#1187](https://github.com/github/view_component/pull/1187) (@BlakeWilliams)
- Remove broken `files` options from vale config [\#1186](https://github.com/github/view_component/pull/1186) (@Spone)
- Improve instructions on how to run tests [\#1185](https://github.com/github/view_component/pull/1185) (@Spone)
- Add `--locale` flag to the component generator [\#1183](https://github.com/github/view_component/pull/1183) (@bobmaerten)
- Move changelog entry for \#1162 to the right version [\#1181](https://github.com/github/view_component/pull/1181) (@rmacklin)
- Update ruby to the latest versions [\#1176](https://github.com/github/view_component/pull/1176) (@VSPPedro)
- Add failing test for default form builder [\#1090](https://github.com/github/view_component/pull/1090) (@boardfish)
- \[1029\] Preview the source of templates when exclusively used [\#1047](https://github.com/github/view_component/pull/1047) (@edwinthinks)
- Validate collection parameter with Active Record attribute names [\#971](https://github.com/github/view_component/pull/971) (@boardfish)
For comparison, here's the current changelog:
---
layout: default
title: Changelog
---

# Changelog

## main

* Return correct values for `request.path` and `request.query_string` methods when using the `with_request_url` test helper.

    *Vasiliy Matyushin*

* Improve style in generators docs.

    *Hans Lemuet*

* Correctly type Ruby version strings and update Rails versions used in CI configuration.

    *Hans Lemuet*

* Make `ViewComponent::Collection` act like a collection of view components

    *Sammy Henningsson*

* Update `@param` of `#render_inline` to include `ViewComponent::Collection`.

    *Yutaka Kamei*

## 2.47.0

* Display preview source on previews that exclusively use templates.

    *Edwin Mak*

* Add a test to ensure trailing newlines are stripped when rendering with `#render_in`.

    *Simon Fish*

* Add WEBrick as a depenency to the application.

    *Connor McQuillan*

* Update Ruby version in `.tool-versions`.

    *Connor McQuillan*

* Add a test to ensure blocks can be passed into lambda slots without the need for any other arguments.

    *Simon Fish*

* Add linters for file consistency.

    *Simon Fish*

* Add @boardfish to docs/index.md and sort contributors.

    *Simon Fish*

* Set up Codespaces for bug replication.

    *Simon Fish*

* Add instructions for replicating bugs and failures.

    *Simon Fish*

* Make @boardfish a committer.

    *Joel Hawksley*

* Validate collection parameter with Active Model attribute names.

    *Simon Fish*

* Fix `helpers` not working with component slots when rendered more than 2 component levels deep.

    *Blake Williams*

* Update ruby to the latest versions

    *Pedro Paiva*

* Fix `vale` linter config options.

    *Hans Lemuet*

* Improve Contributing docs to include how to run tests for a specific version on Rails.

    *Hans Lemuet*

* Add failing test for default form builder and documentation around known issue.

    *Simon Fish*

* Add `--locale` flag to the component generator. Generates as many locale files as defined in `I18n.available_locales`, alongside the component.
* Add config option `config.view_component.generate_locale` to enable project-wide locale generation.
* Add config option `config.view_component.generate_distinct_locale_files` to enable project-wide per-locale translations file generation.

    *Bob Maerten*

Note that it's possible to add YAML front matter, but I didn't realise we needed it until after running the script.

I'm aware of a few discrepancies:

  • Hyphens are used for bullet points (not a huge problem)
  • There's less spacing (I don't like that aesthetically)
  • Contributors are referred to by username and not real name (I feel like that makes things less personal)
  • It's highlighted that PR titles don't tend to be as descriptive as their corresponding changelog entries. Review on this front is also more difficult since there's no feature in GitHub to suggest changes to a PR title.
  • Contributors aren't on their own line
  • Release PRs are included in the changelog

However, there's also nothing stopping me (or anyone else) from extending github-changelog-generator to support some of this stuff. I should add that I really like that it adds links to the PRs and diffs as @rmacklin suggested. That's a good improvement.

Here's what I'd suggest:

  • If necessary, I or others contribute to github-changelog-generator to support:
    • adding the contributor's name in place of their GitHub username
    • adding the contributor's name on its own line
    • additional spacing between each entry
  • Going forward, release PRs are tagged with a label that excludes them from the changelog (unless there's a better solution there)
  • The current changelog is moved to HISTORY.md so that it is the basis for the new changelog
  • A GitHub Action is responsible for adding to the changelog after a merge, so that changelog updates happen in isolation and aren't subject to merge conflicts

I might be overcomplicating things a touch, but what do folks think?

@BlakeWilliams
Copy link
Contributor

  • Contributors are referred to by username and not real name (I feel like that makes things less personal)

Happy to have a conversation around this, but personally I'm 👍 on going by username. The thought being that there's several Blake Williams on GitHub but only one @BlakeWilliams. I think another benefit is that omitting real names is one fewer steps contributors have to take in order to contribute (assuming we can't automate it), which I think is a win.

If we still want to use full names, I wonder if we could get it from the commit since that information should be present there.

Overall I'm 👍 on that approach though!

@boardfish
Copy link
Collaborator

That's a fair point. There's also the argument that folks' identities can change with time, and their username is the closest thing we've got to a unique identifier that will follow them with time. It does give me a lot of personal satisfaction to see my name as opposed to my username in there, but I think it would be a sensible decision to move from that.

@rmacklin
Copy link
Collaborator Author

rmacklin commented Jan 31, 2022

I considered a GitHub Action, too, but I think handling it in the release script might be simpler and cleaner - particularly since the release script is already modifying the changelog.

Apologies that it took me a while to get back to this, but I've drafted a PR outlining this approach in #1259. It could still use a bit of polish but I wanted to push it up to illustrate the approach more concretely. Curious to get your thoughts on it - personally, I like it more than having a bot generate a commit to update the changelog between every real commit, and I think:

Unreleased changes wouldn't appear in the changelog, though we could include a link to those at the top of the changelog

is a fine tradeoff.

@boardfish
Copy link
Collaborator

Unreleased changes wouldn't appear in the changelog, though we could include a link to those at the top of the changelog

#1247 was the sort of issue that could quickly be resolved with an overview of unreleased changes in changelog.md - admittedly, I didn't look there first on that issue, but I feel like it's a good signal that that's a feature we could do with preserving.

I think you're right that the commit history shouldn't be plagued with changelog updates, but I still feel that the changelog should be updated after every PR that goes in. If there's a way to do that without generating a commit against main, then that'd be a strong improvement.

@joelhawksley
Copy link
Member

👋🏻 reading this through, my main take away is that we might just be shifting the burden of contribution from the changelog to the writing of commit messages/PR titles.

Given this discussion, I wonder if we might just want to remove the changelog build requirement and leave it as a suggestion in the contributing docs. We can always ask folks to add an entry if they have omitted one for a material change.

I really appreciate the discussion here and the proposed PRs ❤️

@boardfish
Copy link
Collaborator

I think that could be a quick way to improve the current situation. The problem we're trying to solve is that updating the changelog isn't a frictionless process, since it's subject to merge conflicts. I'll admit that my first shot at solving this was perhaps a little heavy-handed with that in mind.

With that in mind, I think the most direct way to solve this in full is to ensure that a rebase, followed by a changelog update, is the last thing that happens before a merge. Maybe what that means is the current requirement for a changelog update should stay, but that it should only be enforced by maintainers after a PR is otherwise ready to go?

@rmacklin
Copy link
Collaborator Author

rmacklin commented Feb 4, 2022

my main take away is that we might just be shifting the burden of contribution from the changelog to the writing of commit messages/PR titles

I think it's specifically PR titles rather than commit messages (for a single-commit PR, the commit message subject does get auto-filled as the default PR title, but it can be changed). I actually considered this shift as a benefit because it's very easy (for contributors and maintainers alike) to change a PR title. In the past, I've seen maintainers ask for modifications to the changelog entry, which requires a new commit that consequently enqueues the whole CI matrix again. With the alternative approach, that could be avoided: If the only thing left to do before merging is reword the change summary, a reviewer could edit the PR title and then click merge in a matter of seconds. Additionally (and perhaps more significantly), if changelog entries are sourced from PR titles, these issues would no longer exist:

  • CHANGELOG.md is a frequent source of merge conflicts among the PRs that aren't merged in a short window.
  • It's possible to miss an entry in the GitHub Release description because a change was listed under the wrong version when merging CHANGELOG.md. (Move changelog entry for #1162 to the right version #1181)

And I think changelog automation would also be able to solve for this other issue:

(which I didn't include in my initial PR, but is a simple addition) as well as the add-on benefit of including PR numbers with each change.

But with all that said, I'm fine if we don't want to pursue changelog automation. Especially given this suggestion:

I wonder if we might just want to remove the changelog build requirement and leave it as a suggestion in the contributing docs. We can always ask folks to add an entry if they have omitted one for a material change.

because if we are no longer requiring entries for changes that aren't material to the actual runtime, that would at least reduce the potential for merge conflicts. And like I was saying in #1182 (comment), the repository's commit log and contributor data would still provide recognition for each contribution. But I agree with @boardfish that it may be beneficial to update the contributing guidelines to suggest a changelog update as the final step after the branch has been rebased on (or merged with) the latest main, to further minimize conflicts and errors in changelog placement.

@boardfish
Copy link
Collaborator

boardfish commented Feb 4, 2022

If we do still want to automate this element, I have a potential solution with the above in mind. This'll be a workflow, and we have the option to make it required or not.

  1. Fail if no changes have been written to the changelog.
  2. If triggered manually, the PR title will be written to the top of the changelog under the main section. All assignees are credited, using their GitHub handles as @/BlakeWilliams previously suggested.
  3. We'll continue to use the release script to replace main with the new version number.

The criteria for triggering it manually could be one of several things:

  • PR approval
    I wouldn't recommend this, as approvals can still be laced with potential changes that the author may wish to make.
  • A comment matching some string
    Not sure about this. It'd be another thing for contributors to remember.
  • A reaction to a bot comment
    This would probably be the best option I can think of, as the comment itself could document what contributors need to do. I can picture it saying something like "If your PR has been approved and you're ready to merge this PR, react to this comment to add your change to the changelog."

I also contemplated doing it just before a merge, but I'm not sure that's possible without tampering with main or development branches for a brief period of time and creating a window for conflicts. The closest event GitHub exposes is the pull_request.closed event, which takes place after the merge has already happened, so we can't change what's on the branch before it goes out.

@Spone
Copy link
Collaborator

Spone commented Feb 14, 2022

I haven't spent as much time studying this as you have, but I just found this https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes#configuration-options

I'm wondering if there is a way to leverage this configuration (which is based on labels) to determine which changes go to an automatically generated changelog, and maybe "editorialize" the changelog as well (with headings such as Breaking changes, Added, Deprecated, Experimental...)

@joeldrapper
Copy link

The change log doesn't need to be a file. Could just point to the releases page https://github.com/ViewComponent/view_component/releases

@joelhawksley
Copy link
Member

@joeldrapper I'm hesitant to not have something in source control, as it allows us to require PR authors to provide a changelog entry. As of now I'd be most keen on using something like https://github.com/changesets/changesets, albeit with different release notes styling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants