-
Notifications
You must be signed in to change notification settings - Fork 2k
Conversation
e9a78b6
to
ea8c03d
Compare
@codydaig we should also update the contributing.md to follow some sort of commit message structure like the one found here https://github.com/stevemao/conventional-commits-parser. |
@ilanbiala I think the commit messages should be a separate PR. That's something that should be open for discussion on the formatting. |
@codydaig if we're using this package, there's relatively little discussion because it requires a base format and I don't see why we should bother coming up with our own format when we can just use a proven one like Angular's. |
LGTM I like this Changelog format better than anything we had so far. |
@codydaig you simply created the changelog by running an npm package, right? did it require any base format to work on? |
@lirantal That's all I did was run the command line tool. |
@lirantal for it to work much better, we have to standardize a commit message then, so let's establish that in this PR before we merge this in because we may need to change stuff here. |
@ilanbiala commits are harder to control in a sense. The best way would be for a developer/contributor to add some git hook that will enforce a standard, just like we use linters for code, but you obviously realize that this can't be enforced easily sense this is on the contributor's side. Until then, we'll have to tell people to rebase their commits more often to confirm with a standard and it seems to me at this point as maybe too much. |
@lirantal asking people to follow something like:
doesn't seem like too much to ask. |
@ilanbiala I'm not saying it is. Can we provide documentation on the commits guideline? we need to add it in the CONTIRBUTING.md as well as any other place where we mention contribution and be able to easily refer contributors to it in their PRs. That's it. |
@ilanbiala let's mention it wherever required as simple as possible for anyone to understand so this doesn't become a showstopper for contributing. |
@ilanbiala @lirantal I'm cool with enforcing it, but I agree, it needs to be clear and in the Contributing.md doc. Should I add that to this PR or shall we submit a new one? |
@codydaig add to this PR. |
ea8c03d
to
3ad637e
Compare
@lirantal @ilanbiala Updated |
## Commit Message Guidelines | ||
|
||
* Format: | ||
* `<type>(<scope>): <subject>` |
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.
That's just for the header, there can also be a body with more detailed info, and the footer includes all the issues it fixes.
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.
@codydaig can you also clarify with a bit more detail here?
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.
@ilanbiala Can you provide some examples? I don't know what else to put.
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.
Header
Body
Footer
The header should look like:
<type>(<scope>): <subject>
The body should have any necessary detailed info about the commit:
An example, references as to where this idea came from, etc.
The footer should have all the issues tagged:
Fixes #123, Fixes #456
So a commit should like like:
feat(users): Add new Yahoo authentication
Yahoo authentication idea proposed by @codydaig
Example implementation in file.js
Fixes #82
Something like that?
3ad637e
to
6349e39
Compare
@ilanbiala Updated. |
6349e39
to
892d9bf
Compare
@ilanbiala I just made the final changes you requested. Final review before I merge? |
LGTM. Thanks guys! I'm definitely good with the new commit format. It will be a huge help. |
LGTM. |
Generated from:
https://github.com/ajoslin/conventional-changelog
Fixes #881