-
Notifications
You must be signed in to change notification settings - Fork 17
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
Customize commit messages #679
Customize commit messages #679
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!
4b08091
to
cbc33d0
Compare
Thanks for the review @ChrisCarini. I've rebased my branches onto main to resolve the conflicts. |
cbc33d0
to
608605d
Compare
Rebased once more to resolve conflicts in |
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! @cristiangreco , thoughts? 😄
Hi @cristiangreco, I hope all is well 😊 Just checking if there is anything I can improve here to get this merged. I would appreciate any feedback. If you just haven't gotten around to take a look that's of course fine too. |
will be very useful for me if this also supports PR title |
I can look into that over the weekend. It might become it's own PR though, to keep the changes limited in their scopes. |
FWIW (I'm not the owner of this repo), it'd make sense to do that in a separate change. I personally think this change is ready for an approval and to be merged+released, so I wouldn't want to delay that on a separate (although, very related) feature. @cristiangreco - thoughts? Think we can get this PR merged in and released to folks soon? |
608605d
to
5e4f513
Compare
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! @cristiangreco mind taking a few minutes to review this when you have a moment?
As follow up to #695 (comment) this PR needs some adjustments. I am about to leave for a short vacation and can tackle it some time next week. |
5e4f513
to
58c3265
Compare
Updated this PR to match the nullability behaviour of #695 |
This change adds a new input to the action. It can be used to customize the message of the commit created by this action. This input is not required and has the default value `Update Gradle Wrapper from %sourceVersion% to %targetVersion%`, where `%sourceVersion%` will be replaced by the current/old version of the Gradle Wrapper and `%targetVersion%` will be replaced by the new version of the Gradle Wrapper. The PR title and body remain unaffected by this change. Implements gradle-update#246
79ca8bd
to
1d487cf
Compare
For cases where the source version of the Gradle wrapper cannot be determined the string `undefined` is used as a fallback. This allows to use the same custom commit message template in all cases.
1d487cf
to
65bfe63
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
=======================================
Coverage ? 83.53%
=======================================
Files ? 16
Lines ? 589
Branches ? 82
=======================================
Hits ? 492
Misses ? 97
Partials ? 0 ☔ View full report in Codecov by Sentry. |
@cristiangreco - are you able to take a look at this PR that @paulschuberth has put forth when you have a moment? |
Hi @cristiangreco |
@cristiangreco - are you able to take a look at this PR that @paulschuberth has put forth when you have a moment? |
Hey @cristiangreco - any chance you could take a look at this PR when you have a moment? 🙏 I'd love to see it merged in. |
Hi @cristiangreco - I'm sure you're very busy with other work, but when you have ~5 min, would you mind taking another peak at this PR that @paulschuberth put forward? It's been open ~14 months now, and I'd love to see it reviewed / merged in, if you feel it's ready! |
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.
Thanks for the PR and sorry for keeping you waiting so long!
This change adds a new input to the action. It can be used to customize
the message of the commit created by this action.
This input is not required and has the default value
Update Gradle Wrapper from %sourceVersion% to %targetVersion%
, where%sourceVersion%
will be replaced by the current/old version of the Gradle Wrapper and
%targetVersion%
will be replaced by the new version of the GradleWrapper.
The PR title and body remain unaffected by this change.
Implements #246