-
Notifications
You must be signed in to change notification settings - Fork 370
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
feat(api): Add the ability to include commit authors and PR references to default-changelog-notes #1938
base: main
Are you sure you want to change the base?
Conversation
…- address feedback
import {PullRequestBody} from '../../src/util/pull-request-body'; | ||
import {Version} from '../../src/version'; | ||
const presetFactory = require('conventional-changelog-conventionalcommits'); | ||
|
||
const COMMIT_WITH_AUTHOR_FORMAT = '{{#if authors}} by {{authors}}\n {{~/if}}\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.
Add PR author to a commit entry for the generated release notes.
I wonder if this feature be default or opt-in.
This would be very nice to have! Another related thought, maybe a list of excluded authors? For example, I wouldn't want myself listed by every single commit I do. |
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.
Sorry for the late review, but this is looking great!
One question -- is this opt-in? IMO, all the new features should not change existing behavior.
const changelogNotes = buildChangelogNotes({ | ||
type: options.changelogType || 'default', | ||
github: options.github, | ||
changelogSections: options.changelogSections, | ||
commitPartial, |
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.
This feature is entirely opt-in.
commitPartial
already existed as as optional argument prior to this PR.
It was just never used.
commitPartial?: string; |
So now we provide a real value if commitPartialPath
(newly added as part of this PR) is specified in the manifest file.
Yep. |
Yeah I can see this being annoying. I was thinking we could also remove the Alternatively, I wonder if it is possible to supress the notifications for these Github mentions created by |
What is the current state of this PR? Thanks for your work! |
I have updated the PR again. It should be ready for another review from @chingor13. |
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.
This looks so close. I think we will want to pass the template string in the config, rather than making an extra GitHub API request to fetch the template from a file on the repository. The template contents should be small enough (generally a one-liner)
The example template I used is quite big. Though i'm not too sure how others will use it Are you saying we should have the contents of the 'commit-partial.hbs' file as part of the config? |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Context:
Fixes #1716 🦕
This PR is a follow-up to #1717
We can probably close it as my PR is based off it.
Changes:
commitPartial
attribute fromBaseStrategyOptions
commitPartialPath
to the manifest config and friends.release-please-commit-partial.hbs
file.commitPartialPath
is provided)TESTING
I have done testing on this dummy repo.
https://github.com/jamszh/test-release-please
You can provide a path to a custom
commit-partial.hbs
file like this one../build/src/bin/release-please.js release-pr --token $GITHUB_TOKEN --repo-url jamszh/test-release-please --target-branch main --commit-partial-path .release-please-commit-partial.hbs
Here is an example auto-release PR generated by the command above.