-
Notifications
You must be signed in to change notification settings - Fork 520
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
chore: release 1.0.0 of specification #175
Conversation
I think it would be good to include |
|
@stevemao left a comment on the revert discussion. I think we should explicitly call out |
Hey @bcoe, I can't dive into this too deep right now, but I just noticed you created this PR, and wanted to link you to an issue on a project of mine in which I'm working on a parser for Conventional Commit. I think the contents of that discussion might be interesting to you. Not sure if it'll make any difference in whatever ends up getting merged as rustic-games/conventional-commit#1 (comment) (the parser is used in another library, Jilu, to auto-generate a change log based on conventional commits, which is why being able to correctly parse breaking changes (and possibly other metadata) is important to get right) |
@bcoe Instead of the # of the PR or MR, I would instead recommend to reference the merge commit or a range of commits or the branch if it still exists. |
@Mouvedia feel free to open an issue and work on a PR about it :D |
@Mouvedia it is totally up to the userland to specify it IMHO. |
> If included in the type/scope prefix, breaking changes MUST be > indicated by a `!` immediately before the `:`. To support this, you can now access `commit.breaking()` to get a boolean signaling if the commit contains breaking changes. You can still access `commit.breaking_change()` to get any optional message provided using the `BREAKING CHANGE: ` paragraph. If no `!` is provided, but the commit does contain a `BREAKING CHANGE` paragraph, the `commit.breaking()` method will also return `true`. While the API is a bit awkward right now due to having `breaking` _and_ `breaking_change`, this is only temporarily, until Git trailers are implemented, and the special `breaking_change` method will be removed in favor of a generic map of trailer key/values with one of those trailers having the key `BREAKING CHANGE`.
@damianopetrungaro I think @JeanMertz has put together a very sound proposal for eliminating ambiguity around body vs., footer. @JeanMertz, perhaps make a PR against this branch adding some of your edits? I'm less convinced that we need to strictly define the behavior around revert, beyond giving some sound recommendations in the FAQ. |
@Mouvedia @stevemao I've added an FAQ section providing suggestions for handling reverts, based on the angular convention. For releasing
@tommywo @damianopetrungaro I think we've done a good job of word-smithing the relationship between
@epage @JeanMertz, with your help I think we've done a much better job of making the difference between footers and body well defined. So what do folks say, shall we get this to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice improvements to the spec. Looking forward to seeing this merged. I left a few comments, but nothing blocking.
content/next/index.md
Outdated
@@ -2,7 +2,7 @@ | |||
draft: true | |||
--- | |||
|
|||
# Conventional Commits 1.0.0 | |||
# Conventional Commits 1.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about the patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ 1️⃣ ✨
Haven't forgotten about this, have just been swamped 😆 @damianopetrungaro you're comfortable with landing this once I address comments? |
@bcoe sure I am! Thanks for pushing it forward! 💪 |
Ahhhhhhh... Sadly, couldn't get sooner to that, nor pinged... Just was reading it when you merged it and was going to point a few things:
|
@tunnckoCore if you look at the final edit that landed, we refer to things as "multiple footers" not as trailers (we only reference tailers a couple times, pointing out that they inspired the footer parser). For the other minor changes, send a patch and we can release |
@tunnckoCore I think a better example would be Co-authored-by:. |
Exactly, that's my proble 😆 I don't like it and sounds me strange. It would be better to just follow the git trailers language: "Multiple trailers inside a single footer. And what's a footer? Well, it starts where the first trailer starts. What's body? It begins after the first line, and ends where the footer starts (first found trailer)".
Yep.
Agree, haha. |
@tunnckoCore I think we're different enough from trailers, that I kind of agreed with @JeanMertz's recommendation to not tie us to them too tightly. |
I don't see any difference. Each token/value pair is a trailer. What's the difference between "multiple footers" and "git trailers"? In abstract world, they are childs of a footer.
* "subject" is officially called Example commit message:
Parsed to object: const commit = {
header: {
type: 'feat',
scope: null,
subject: 'meow meow, yawn'
},
body: 'foo qux\n\nbar\nvar\n\nzero barry\ndeal',
footer: {
trailers: [
'Reviewed-by: Zzz oops',
'Some-thing: Multiline important\n\ntrailer woohoo\nmeow meow\n\nWhy not?\nyup!',
'Signed-off-by: Foo <bar@qux.com>'
],
trailer: {
'Reviewed-by': 'Zzz ooops',
'Some-thing': 'Multiline important\n\ntrailer woohoo\nmeow meow\n\nWhy not?\nyup!',
'Signed-off-by': 'Foo <bar@qux.com>',
},
raw: `Reviewed-by: Zzz oops
Some-thing: Multiline important
trailer woohoo
meow meow
Why not?
yup!
Signed-off-by: Foo <bar@qux.com>`,
},
}; In the |
@tunnckoCore could you please open several issues for these conversations, rather than commenting on a closed pull request? |
The current implementation accepts compound nouns as types, but the first version of the spec is about to be released[1], and explicitly requires the type to be a noun. [1]: conventional-commits/conventionalcommits.org#175
I would like to advocate releasing version
1.0.0
of Conventional Commits.Reasoning
!
(which is working great) there haven't been other major updates).Let's Practice What We Preach
Releasing a
1.0.0
version of the specification doesn't mean that we can't iterate, it just means that we drop the uglybeta
/alpha
suffix and start following SemVer for updates to the specification.CC: @tommywo, @jbottigliero