-
-
Notifications
You must be signed in to change notification settings - Fork 297
Feature/allow empty commit #592
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
Feature/allow empty commit #592
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## master #592 +/- ## ======================================= Coverage 98.32% 98.33% ======================================= Files 39 39 Lines 1614 1618 +4 ======================================= + Hits 1587 1591 +4 Misses 27 27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@Lee-W Can you elaborate about what bother you in this test case ?
I'm kinda not sure whether this is correctly tested and will need sometime to figure it out. Will try to check it this week
Hi, I think we have a misunderstanding here. When I say "Empty commit" I mean a commit with no changes in it but we still need a message inside.
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.
@Dranaxel Thanks for correcting me. Yes, you're right. In that case, we probably should assert whether this function is called with --allow-empty. This can be done by something likeassert_called_with.
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 i'm not sure about what you mean/how to use assert_called_with. Are you suggesting to assert the flag allow-empty was here when le command was launched ?
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.
assert --allow-empty is actually passed into commitizen.git.commit when the operation above is executed
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.
Need rebase but I am OK with this one. I can definitely see use cases where you just want to force a commit without changes (like forcing rebuild to have updated dependencies).
Note: we might want the bump counter part for the same reason, aka. being able to force a release not containing commits eligible to changelog (aka. #723)
Commitizen has cz commit -- --allow-empty now. Do we still need this change?
Commitizen has
cz commit -- --allow-emptynow. Do we still need this change?
Yep, you're right. Thanks for reminding us. Let me close it
Uh oh!
There was an error while loading. Please reload this page.
Description
Add allow to create empty commit
Checklist
./scripts/formatand./scripts/testlocally to ensure this change passes linter check and testExpected behavior
Allow to invoke cz and create a commit without any change in it
Additional context
Relates to #247 & #590