-
-
Notifications
You must be signed in to change notification settings - Fork 300
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: generate changelog based on conventional commits #5487
Conversation
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.
LGTM!
Performance Report✔️ no performance regression detected Full benchmark results
|
scripts/generate_changelog.js
Outdated
/** | ||
* @type {Record<string, {heading: string; commitsByScope: Record<string, string[]>}>} | ||
*/ | ||
const sections = { |
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.
Looking for feedback on what sections, headings and ordering we want to use here.
scripts/generate_changelog.js
Outdated
continue; | ||
} | ||
|
||
const [, type, scope, _breaking, subject] = conventionalCommit; |
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.
Breaking changes are not highlighted at the moment, possible solution could be to have an extra section ## Breaking Changes
but based on how we used !
so far this would be too noisy as there are too many changes marked as breaking which are not user facing.
I think if we can restrain breaking commits as highlighted by !
to user facing changes it would be valuable as it provides a quick way for operators (or other downstream tooling) to just check this section to know if they are affected by any of the introduced changes.
Potential solutions to highlight breaking changes
- separate Section (
## Breaking Changes
) - prefixing or suffixing line (
**BREAKING CHANGE:**
or[BREAKING]
) - using some kind of emoji at beginning of the line (
⚠️ )
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.
I have no strong opinion to either method. However as you already mentioned, we do have a lot of breaking changes which isn't necessarily useful to a non-developer user/operator of Lodestar. If it does affect a user, it'll likely be highlighted already on my manual summaries.
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.
Let's not highlight breaking changes in the Changelog for now, until we have more clarity on it. Updated description of #5433 to discuss it there.
/** | ||
* @type {Record<string, {heading: string; commitsByScope: Record<string, string[]>}>} | ||
*/ | ||
const sections = { |
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.
Order and wording LGTM
const isPrCommitRg = /\(#\d+\)/; | ||
const conventionalCommitRg = /^([a-z]+)(?:\((.*)\))?(?:(!))?: (.*)$/; |
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.
Thats pretty a pretty fancy regexp. Did this get pulled from somewhere? Perhaps we can unit test it but that feels silly honestly for a workflow script...
Found this thread and it had a a pretty cool one in it to support emojis.
Not something I feel strongly about but thought was worth a mention. I put my faith in your trusting hands sir
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.
Not in favor of adding tests for the script. I have verified that the output is correct.
As I also do not trust any regex, I created some test cases which I just executed locally.
a pretty cool one in it to support emojis
The regex allows emojis in the commit message itself. It is really not that strict, a more strict format will be enforced by the PR title checker.
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.
Added console logs to print out ignored commits (dc26df1), this should be sufficient for now to verify that the regex works and there are no false positives.
const authorEmail = shell(`git log --format='%ae' ${commitHash}^!`); | ||
const authorName = shell(`git log --format='%an' ${commitHash}^!`); | ||
const login = getCommitAuthorLogin(commitHash, authorEmail, authorName); | ||
|
||
commitListStr += `- ${subject} (@${login})\n`; | ||
const formattedCommit = `- ${scope ? `**${scope}:** ` : ""}${subject} (@${login})\n`; |
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 touch looking up the author login name!! Always great to provide visibility to contributors
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.
Yeah, it is pretty nice, can easily see contributors of a release. That was already there in our initial version and does not seem to be easily supported by any other tools like release-please or semantic-release which is another good reason besides added complexity to not introduce additional tooling (for now).
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.
Excellent work! A couple of really nice touches in there!
🎉 This PR is included in v1.9.0 🎉 |
Motivation
More structured and readable Changelog.
Part of #1773 and follow up to #5342 which already enforces conventional commit PR titles.
Description
Running the following command will produce this CHANGELOG.md
Related resources