Skip to content
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(commit): add '--allow-empty' flag to commit command #1217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AdrianDC
Copy link
Contributor

Description

feat(commit): add '--allow-empty' flag to commit command

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

Allow to invoke cz and create a commit without any change in it

Steps to Test This Pull Request

git cz c --allow-empty

Additional context

@AdrianDC AdrianDC changed the title Allow empty feat(commit): add '--allow-empty' flag to commit command Aug 22, 2024
Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.58%. Comparing base (120d514) to head (a442d70).
Report is 492 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1217      +/-   ##
==========================================
+ Coverage   97.33%   97.58%   +0.24%     
==========================================
  Files          42       55      +13     
  Lines        2104     2608     +504     
==========================================
+ Hits         2048     2545     +497     
- Misses         56       63       +7     
Flag Coverage Δ
unittests 97.58% <100.00%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Lee-W
Copy link
Member

Lee-W commented Aug 22, 2024

Hi, thanks for contributing! would like to know why do we want --alow-empty. I thought we already have -- --allow-empty

@AdrianDC
Copy link
Contributor Author

Coverage fixed 👍.

Lee-W, apart from regular usage, it's because the staging area checks fail :

 $ cz c --allow-empty
-Invalid commitizen arguments were found: `--allow-empty`. Please use -- separator for extra git args

 $ cz c -- --allow-empty
-No files added to staging!

 # =======================
 $ pipx uninstall commitizen; pipx install .
 uninstalled commitizen! ✨ 🌟 ✨
   installed package commitizen 3.29.0, installed using Python 3.12.3
   These apps are now globally available
     - cz
     - git-cz
 done! ✨ 🌟 ✨

 # =======================
 $ cz c --allow-empty
 ? Select the type of change you are committing feat: A new feature. Correlates with MINOR in SemVer
 ? What is the scope of this change? (class or file name): (press [enter] to skip)
  test
 ? Write a short and imperative summary of the code changes: (lower case and no period)
  test
 ? Provide additional contextual information about the code changes: (press [enter] to skip)
  
 ? Is this a BREAKING CHANGE? Correlates with MAJOR in SemVer No
 ? Footer. Information about Breaking Changes and reference issues that this commit closes: (press [enter] to skip)
 

 feat(test): test


 [master 6b9618c5] feat(test): test

+Commit successful!

@AdrianDC
Copy link
Contributor Author

As discussed, I redid it all into -- --allow-empty support + implemented matching tests 👍.

Tested locally with cz c, cz c -- --allow-empty and cz c -s --allow-empty

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like part of the code is also used in #1206. let's merge #1206 first and rebase that into this one and then I'll take a final review.

@AdrianDC
Copy link
Contributor Author

Rebased, ready to roll after 1206 👍

@Lee-W
Copy link
Member

Lee-W commented Nov 16, 2024

1206 merged 🙌

@AdrianDC
Copy link
Contributor Author

👍

@Lee-W Lee-W self-requested a review November 20, 2024 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants