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

ci: Group changelog entries for breaking changes #2019

Merged
merged 1 commit into from
Apr 5, 2022

Conversation

helderco
Copy link
Contributor

@helderco helderco commented Apr 4, 2022

Overview

If there's a change that requires migrations from users, an easy thing we can do right now is to make them clear in the changelog. In Python, I'm used to read the release notes for packages before updating them.

The convention, according to https://www.conventionalcommits.org is to add a ! before the ::

  • chore!: Move packages to core
  • feat!: Rename field X

RFCs

  1. I thought about using break: instead, but decided to go with the convention above in case we decide to adhere more to it later. WDYT?

  2. Not sure about the title for the "other" group: Changes. We need to consider there will be logs without breaking changes, so it needs to make sense on its own.

  3. We could also group by features and bug fixes, but I think we need to have the discipline to add these prefixes to all commits for that (basically adhering to the conventions above).

Signed-off-by: Helder Correia

This gives visibility into breaking changes so that one can look to the changelog to know how to adapt.

Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
@helderco helderco requested a review from a team as a code owner April 4, 2022 11:47
@helderco helderco changed the title ci: Group changelog entries ci: Group changelog entries for breaking changes Apr 4, 2022
@gerhard
Copy link
Member

gerhard commented Apr 4, 2022

If there's a change that requires migrations from users ... according to https://www.conventionalcommits.org is to add a ! before the ::

  • chore!: Move packages to core
  • feat!: Rename field X

Big yes to this!

  1. I thought about using break: instead, but decided to go with the convention above in case we decide to adhere more to it later. WDYT?

These conventions are sensible, I also think we should stick to them.

  1. Not sure about the title for the "other" group: Changes. We need to consider there will be logs without breaking changes, so it needs to make sense on its own.

I would not add the third category. Users care about fixes & features, the rest is noise. Instead, I would add a link to the full changelog which should cover everything, e.g. https://github.com/goreleaser/goreleaser/releases/tag/v1.6.0

Here's another good example: https://github.com/cli/cli/releases/tag/v2.7.0

  1. We could also group by features and bug fixes, but I think we need to have the discipline to add these prefixes to all commits for that (basically adhering to the conventions above).

Yes, we should absolutely do this.

We should also add list of contributors & full changelog (see the examples above). I have been using the full changelog for years, and find it extremely helpful when looking for unintended side-effects - found a bad one in PostgreSQL in 2019.

@helderco
Copy link
Contributor Author

helderco commented Apr 4, 2022

  1. Not sure about the title for the "other" group: Changes. We need to consider there will be logs without breaking changes, so it needs to make sense on its own.

I would not add the third category. Users care about fixes & features, the rest is noise.

Not what I meant. There's two with this PR: 1) breaking changes; 2) other changes (title "Changes"). I'm referring to the catch-all.

[...] I would add a link to the full changelog which should cover everything

Good point, I will add that.

@TomChv
Copy link
Member

TomChv commented Apr 4, 2022

We could also group by features and bug fixes, but I think we need to have the discipline to add these prefixes to all commits for that (basically adhering to the conventions above).

So ensure that we always follow the convention, we can use a GitHub action.
After a quick research, I found that one : https://github.com/webiny/action-conventional-commits and we can use https://github.com/commitizen/cz-cli or an extension for the format.

Indeed, I think we could also automatically add label according to file changed or commit/PR prefix

Copy link
Member

@aluzzardi aluzzardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Left a clarification comment

@@ -53,6 +53,12 @@ changelog:
- "^infra:"
- "^build\\(deps\\):"
- "^Merge pull request"
groups:
- title: Breaking Changes
regexp: "^.*!:+.*$"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, this will put anything!: inside Breaking Changes.

Is the convention to call them break!: ...?

Copy link
Contributor Author

@helderco helderco Apr 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention is the exclamation point, no matter the prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be fix!: or feat!:.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is Conventional Commits, correct?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change

A link to the CC site could be helpful 🙂

Copy link
Contributor Author

@helderco helderco Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a footnote to it in the contributing guidelines, and I also mentioned it in the description of this PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, don't know how I missed that. Actually, I think I came from the changelog link to the commit so my context was the commit, rather than the pull request that contained it. That's good to know.

@helderco helderco requested a review from gerhard April 5, 2022 13:22
@aluzzardi aluzzardi merged commit bfdbc9b into dagger:main Apr 5, 2022
@helderco helderco deleted the goreleaser-break branch April 6, 2022 10:35
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 this pull request may close these issues.

None yet

5 participants