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

[changelog] pickup BREAKING CHANGE: from footer #809

Closed
gotson opened this issue Apr 8, 2022 · 18 comments · Fixed by #1087
Closed

[changelog] pickup BREAKING CHANGE: from footer #809

gotson opened this issue Apr 8, 2022 · 18 comments · Fixed by #1087
Assignees
Labels
enhancement New feature or request released Issue has been released
Milestone

Comments

@gotson
Copy link
Contributor

gotson commented Apr 8, 2022

Is your feature request related to a problem? Please describe.
In the conventional commit specification, breaking changes can be specified either with a ! in the title, or with a BREAKING CHANGE: message within the body.

BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in Semantic Versioning). A BREAKING CHANGE can be part of commits of any type.

Example commit.

Examples from conventionalcommits.org

Describe the solution you'd like
Pickup those changes and highlight them as breaking changes.

Describe alternatives you've considered
Tools like semantic-release can pick those up, example here.

Additional context
Add any other context or screenshots about the feature request here.

@gotson gotson added the enhancement New feature or request label Apr 8, 2022
@aalmiray aalmiray added this to the v1.1.0 milestone Apr 8, 2022
@aalmiray aalmiray removed this from the v1.1.0 milestone Jun 20, 2022
@gotson
Copy link
Contributor Author

gotson commented Nov 23, 2022

Hi @aalmiray , in addition of picking up commits with a BREAKING CHANGE: line, but no ! in the first line, the content of the BREAKING CHANGE: message should be picked up and shown in some way.

Example in xerial/sqlite-jdbc@0f41f46 and the generated release notes

The breaking change is not shown, and one must check the commit message to know what's breaking.

An example from the Conventional Commits website shows that you can have the actual breaking change described in the footer, while the commit title is somehow different.

feat: allow provided config object to extend other configs

BREAKING CHANGE: extends key in config file is now used for extending other config files

That makes sense to me for commits of the type build(deps) where you would for example upgrade a dependency, and it has a breaking change. You would document the breaking change in the BREAKING CHANGE: footer, while the commit title would show what version was upgrade from/to.

@aalmiray
Copy link
Member

Alright. I may be able to pick up the footer and add a ! to the rendered title as a result. However, any additional text would have to be wither left in the commit's body. Optionally it could be made available through a name template that you could explicitly set as part of the commit's format. WDYT?

@gotson
Copy link
Contributor Author

gotson commented Nov 23, 2022

You mentioned customizing the templates before, but i'm not really sure how to do that.

Since the above is part of conventional commit specification, and one can choose the changelog format as conventional commit, it should probably apply by default instead of requiring customization.

However, any additional text would have to be wither left in the commit's body

Not sure to understand what that mean exactly.

@aalmiray
Copy link
Member

Ha! Sorry, the word "wither" is a typo. Changelog templates may be customizable in the future, just not yet. Also, JReleaser supports CC but it's not aware of CC itself that is, the preset simply applies a predefined set of rules for labelers & categories but it does not inform JReleaser in any way that CC is in effect. This is perhaps another thing that could be changed, that way changelog templates and the inner plumbing can enforce additional rules pertaining to CC or any other presets/conventions.

@gotson
Copy link
Contributor Author

gotson commented Nov 23, 2022

I think that ticket has actually 2 requirements:

  1. apply the same 🚨 for a commit if BREAKING CHANGE: is in the commit body, same as if there was a ! following the scope
  2. pull out the content of the BREAKING CHANGE: and display it somewhere in the release note, however i am not sure where it should be shown.

Let me know if you want to break 2 into a separate issue.

@aalmiray
Copy link
Member

Correct. There are at least 2 issues as you aptly listed them. Additionally, treat CC (and gitmoji by extension) as proper entities instead of a bolt-on option as they are right now.

@gotson
Copy link
Contributor Author

gotson commented Nov 28, 2022

@aalmiray i was looking at the code (for the first time) to see if i could help in sending a PR, but most likely if you want to make CC and gitmoji first class citizen, that may need some more thoughts. If you have some ideas you want to throw in i would be happy to try.

The way i see it currently, we would probably need more properties in ChangelogGenerator.Commit to store whether it's a breaking change, and also the content of the BREAKING CHANGE: line(s). Then we could add those properties in asContext which is passed to the mustache template, and the formatting would be done there.

If we want CC/gitmoji as entities it's a bit more complex though.

@aalmiray
Copy link
Member

@gotson that's right. ChangelogGenerator.Commit requires additional properties as you correctly pointed out. I'd suggest adding 3 properties to begin with:

  • String breakingChangeContent: holds the content of the BREAKING CHANGE:. Given that the footer should not exist without content is enough to check if this property is blank or not to assert if there's a breaking change or not.
  • Set<String> issues: set of referenced issues in the commit's message.

@gotson
Copy link
Contributor Author

gotson commented Nov 28, 2022

  • Given that the footer should not exist without content is enough to check if this property is blank or not to assert if there's a breaking change or not.

Not necessarily true, you can use the ! marker after the commit type to indicate a breaking change, without having content.

@aalmiray
Copy link
Member

aalmiray commented Nov 28, 2022

A ! marker is not the same as a BREAKING CHANGE: footer syntactically. They are 2 different elements that indicate a breaking change. The ! has not additional content whereas the footer must have one, hence why the breakingChange property is set as a String.

If it were a boolean then it would indicate the commit contains a breaking change regardless of how it was identified as such. We don’t do that yet as there was no need to make it explicit. However, allowing changelog customizations via templates might need this thus capturing if a commit is a BC no matter how it’s signaled may be the way to go, in which case 2 properties are needed: one signal a BC, another to hold the BC message (when available).

@gotson
Copy link
Contributor Author

gotson commented Nov 28, 2022

I have the same understanding as you, and I think having a boolean property would help to flag the 🚨 when the ! is not present.

I may have a go at this and see if I can get it working.

@aalmiray
Copy link
Member

Alright. I think the following should happen:

  • add the 3 afore mentioned properties
  • commit parser must inspect the commit to find out the values. For BC it only makes sense to check if changelog.preset = conventional-commits. Issue extraction is already sone somewhere else in ChangelogGenerator thus the code may be refactored & reused.
  • when a commit is deemed a BC then its title must be updated to contain a ! if it does not have it already. This signals the replacer. This could be refactored later with a CCHandler once changelog template extensions are put in place.

@gotson
Copy link
Contributor Author

gotson commented Nov 29, 2022

Hi @aalmiray ,

i haven't found test code for the actual changelog generation. I could setup some tests for the parsing, but i would need to also test the resulting changelog when playing with templates.

Would you know how i can test that, even if it's manual ?

It's also worth to note that the BREAKING CHANGE: content could be multi-line according to the conventional commit spec, however that would be a bit more complex to support, as we would probably need a parser rather than a regex. For the moment i will only implement single line content, and we can alway improve from there.

@aalmiray
Copy link
Member

We have a test that relies on lots of mocks to make it work. I think that’s the wrong approach at this point. The alternative is to create a sample Git repository (a real one) and use it as a resource. Setting up the test is a bit troublesome at the moment as only a single .git dir may be present in a Git repo, thus sample projects would have to be provided as zip files, unzipped before the testcase is run.

For the time being manual testing is performed on repositories that exhibit the desired conditions. Just running jreleaser changelog should be enough.

@gotson
Copy link
Contributor Author

gotson commented Dec 2, 2022

I've made good progress this week, I now have a ConventionalCommit subclass that has properties matching the conventional commit specification, and unit tests covering all the examples on the official conventional commit website.

Now I need to tweak the default conventional commit preset to use those properties for better flexibility.

@gotson
Copy link
Contributor Author

gotson commented Dec 5, 2022

@aalmiray i have almost everything ready, but i have a small issue. I want to change the default changelog format for the conventional commit preset, but i can't find a way to do so.

In the current codebase, preset-conventional-commits.yml can't define a format entry, because it is not loaded by BaseReleaserValidator.loadPreset.

If i add a line changelog.setFormat(loaded.getFormat());, it would work, but it doesn't allow for overriding the changelog.format in the user's configuration file. I suppose that's because the preset is loaded after the user's configuration is loaded, and thus overrides the default.

Would you have some hints/advice on how i can solve that ?

@gotson
Copy link
Contributor Author

gotson commented Dec 5, 2022

oh, i managed to find a way :)

gotson pushed a commit to gotson/jreleaser that referenced this issue Dec 5, 2022
Introduces a new ConventionalCommit subtype of Commit, which parses the commit body according to conventional commit specification.
The conventional-commit preset uses a custom changelog.format instead of replacers to achieve formatting.
The changelog presets can now have a custom format.

Closes: jreleaser#809
gotson pushed a commit to gotson/jreleaser that referenced this issue Dec 5, 2022
Introduces a new ConventionalCommit subtype of Commit, which parses the commit body according to conventional commit specification.
The conventional-commit preset uses a custom changelog.format instead of replacers to achieve formatting.
The changelog presets can now have a custom format.

Closes: jreleaser#809
gotson added a commit to gotson/jreleaser that referenced this issue Dec 5, 2022
Introduces a new ConventionalCommit subtype of Commit, which parses the commit body according to conventional commit specification.
The conventional-commit preset uses a custom changelog.format instead of replacers to achieve formatting.
The changelog presets can now have a custom format.

Closes: jreleaser#809
gotson added a commit to gotson/jreleaser that referenced this issue Dec 6, 2022
Introduces a new ConventionalCommit subtype of Commit, which parses the commit body according to conventional commit specification.
The conventional-commit preset uses a custom changelog.format instead of replacers to achieve formatting.
The changelog presets can now have a custom format.

Closes: jreleaser#809
gotson added a commit to gotson/jreleaser that referenced this issue Dec 6, 2022
Introduces a new ConventionalCommit subtype of Commit, which parses the commit body according to conventional commit specification.
The conventional-commit preset uses a custom changelog.format instead of replacers to achieve formatting.
The changelog presets can now have a custom format.

Closes: jreleaser#809
@aalmiray aalmiray added this to the v1.4.0 milestone Dec 6, 2022
aalmiray added a commit to jreleaser/jreleaser.github.io that referenced this issue Dec 6, 2022
aalmiray pushed a commit that referenced this issue Dec 6, 2022
Introduces a new ConventionalCommit subtype of Commit, which parses the commit body according to conventional commit specification.
The conventional-commit preset uses a custom changelog.format instead of replacers to achieve formatting.
The changelog presets can now have a custom format.
aalmiray added a commit to jreleaser/jreleaser.github.io that referenced this issue Dec 13, 2022
@aalmiray
Copy link
Member

@aalmiray aalmiray added the released Issue has been released label Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released Issue has been released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants