conventional-commits / conventionalcommits.org

The conventional commits specification
https://conventionalcommits.org
MIT License
7.02k stars 544 forks source link

Breaking change footer together with other footers #413

Open mainrs opened 2 years ago

mainrs commented 2 years ago

The git trailer specification says that each pair of git trailers is separated by a newline only. Does that imply that, if a commit contains a breaking change, the footers have to come directly after it, without an empty newline after it?

This would be wrong:

fix!: parser number handling

BREAKING CHANGE: The parser now treats floats as ints

Signed-off-by: me

This is correct:

fix!: parser number handling

BREAKING CHANGE: The parser now treats floats as ints
Signed-off-by: me
bcoe commented 2 years ago

I was picturing that this would work, but the BREAKING CHANGE would actually be parsed out of the body rather than from the footers.

mainrs commented 2 years ago

I was picturing that this would work, but the BREAKING CHANGE would actually be parsed out of the body rather than from the footers.

Strictly speaking, yes. A lot of repositories that follow CC do use the first notation. Probably to make it more obvious. But it does require more lenient parsing to parse correctly.

Conaclos commented 2 years ago

In my opinion BREAKING CHANGE should not be treated as a trailer. Git does not by default recognize a spaced key as a git trailer. Moreover BREAKING CHANGE sometimes spans on multiple lines. Multi-line git trailers are allowed, but they require a blank character at the beginning of every line and are generally discouraged : git interpret-trailer one-line them.

I personally use the following format:

<type>[scope][!]: <desc>

[body]

BREAKING CHANGE: <desc>

[breaking change body]

[trailers]

If you really want to use a git trailer I could advice you to use "BREAKING-CHANGE" and a single line of description.

<type>[scope][!]: <desc>

[body]

BREAKING-CHANGE: <desc>
[more trailers]
chbi commented 2 years ago

I faced the same issue: "BREAKING CHANGE" does not meet the git trailer convention and breaks our tooling (validation of commit messages using jgit). Our devs interpret "footer" (as used in CC) as "trailer" (as used in Git documentation) and use it that way.

As a workaround, I wrote "our own CC definition", that only allows "BREAKING-CHANGE" as the token in Git trailers.

I suggest one or both of the following:

Concerning Git-Trailers: There is also a discussion on the Git mailing list: https://lore.kernel.org/git/xmqq4jy18q7h.fsf@gitster.g/. Git trailer documentation currently allows whitespace in the token part of trailers, while no major Git implementation does. It appears that git documentation will be fixed and will no longer allow whitespace in git trailers.

relaxdiego commented 1 year ago

It would be good if BREAKING CHANGE: was converted into a well-formed git trailer which complies with RFC 822. Specifically, the meta-syntax 1*<any CHAR, excluding CTLs, SPACE, and ":"> (e.g. BREAKING-CHANGE:) so that it becomes acceptable to a larger set of git users.

Being RFC 822 compliant would also mean breaking changes would show up when certain git log commands are executed. For example:

$ git log --format="%h %(trailers:separator=%x2C )" | \
    grep '^[0-9a-f]* [[:print:]].*'

ac6eee8 BREAKING-CHANGE: Foo no longer bars
e935b07 See: https://cbea.ms/git-commit/#seven-rules
696f1e6 Reference: https://example.com
LemmingAvalanche commented 6 months ago

@Conaclos (fwiw)

Multi-line git trailers are allowed, but they require a blank character at the beginning of every line and are generally discouraged

I haven’t seen it be discouraged.

You can’t have multiple paragraphs though. Like this.

feat!: foo

Fixes: Fusce sagittis, libero non molestie mollis, magna orci ultrices
  dolor, at vulputate neque nulla lacinia eros.

  Nullam eu ante vel est convallis dignissim.  Fusce suscipit, wisi nec
  facilisis facilisis, est dui fermentum leo, quis tempor ligula erat
  quis odio.  Nunc porta vulputate tellus.  Nunc rutrum turpis sed pede.
  Sed bibendum.  Aliquam posuere.  Nunc aliquet, augue nec adipiscing
  interdum, lacus tellus malesuada massa, quis varius mi purus non odio.
  Pellentesque condimentum, magna ut suscipit hendrerit, ipsum augue
  ornare nulla, non luctus diam neque sit amet urna.  Curabitur
  vulputate vestibulum lorem.  Fusce sagittis, libero non molestie
  mollis, magna orci ultrices dolor, at vulputate neque nulla lacinia
  eros.  Sed id ligula quis est convallis tempor.  Curabitur lacinia
  pulvinar nibh.  Nam a sapien.

git interpret-trailer one-line them.

No, only if you explicitly ask for --unfold.