Skip to content

docs: add commit cli #657

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

Merged
merged 3 commits into from
Jan 22, 2023
Merged

docs: add commit cli #657

merged 3 commits into from
Jan 22, 2023

Conversation

12rambau
Copy link
Contributor

Description

Create a dedicated page for the commit command and document the drop of hooks coloring.
Fix #417

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

Steps to Test This Pull Request

check the new doc :-)

@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Base: 97.92% // Head: 98.09% // Increases project coverage by +0.17% 🎉

Coverage data is based on head (5b604e2) compared to base (db42a95).
Patch coverage: 87.69% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
+ Coverage   97.92%   98.09%   +0.17%     
==========================================
  Files          35       39       +4     
  Lines        1252     1682     +430     
==========================================
+ Hits         1226     1650     +424     
- Misses         26       32       +6     
Flag Coverage Δ
unittests 98.09% <87.69%> (+0.17%) ⬆️

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

Impacted Files Coverage Δ
commitizen/commands/init.py 87.93% <84.00%> (-3.74%) ⬇️
commitizen/__version__.py 100.00% <100.00%> (ø)
commitizen/commands/changelog.py 98.86% <100.00%> (ø)
commitizen/commands/check.py 100.00% <100.00%> (ø)
commitizen/exceptions.py 100.00% <100.00%> (ø)
commitizen/__init__.py 100.00% <0.00%> (ø)
commitizen/changelog.py 100.00% <0.00%> (ø)
commitizen/bump.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@12rambau 12rambau marked this pull request as ready for review January 20, 2023 17:41
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.

Thanks @12rambau . most of the part looks good to me , but could you please remove .DS_Store? Thanks!

docs/commit.md Outdated

!!! note
To maintain platform compatibility, the `commit` command disable ANSI escaping in its output.
In particular pre-commit hooks coloring will be deactivated.
Copy link
Member

Choose a reason for hiding this comment

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

I think we can add a link to the original issue.

@12rambau 12rambau requested a review from Lee-W January 21, 2023 13:09
@@ -110,3 +110,6 @@ venv.bak/

# build
poetry.lock

# macOSX
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest we don't add it to project .gitignore. It should be added to somewhere like ~/.gitignore based on platform's need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before removing it let me fight a bit for this modification:

.DS_Store files are generated by mac computer all the time. Lucky me I didn’t execute the lib itself so there is just one but they can appear in any folder. All Mac IDE are hiding them from us meaning that before git add I simply don’t know where they are. If you refuse to add this line to the .gitignore you simply deny to all mac users the possibility to use git add . which can be a real pain when modifying many files.

Let’s have a look at the .gitignore itself. it’s not a « commitizen » specific file but the github template for Python language with small customization. It includes:

  • IDE specific folders (.vscode)
  • sphinx folders (docs/_build/)
  • all the django related files
  • translation files (.mo, .po)
  • scrappy stuff
  • All the distribution files including things unrelated to the project
  • unitesting from the most known tools (coverage, nosetest and tox) even though tox is not part of your lib

meaning that the .gitignore already includes things that no developer will ever need as it’s unrelated to the project. I don’t understand why 1 extra line that will be useful for all mac users seems too much. it’s present in all the open-source projects I contribute in like: opensartoolkit, pydata-sphinx-theme, sepal_ui, pygadm

let me know if even after my little speech you still want me to remove it and I will. 😢

Copy link
Member

Choose a reason for hiding this comment

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

Hi @12rambau thanks for sharing the thought in such detail! tl;dr, you convinced me to accept it in this project.

I just found out we also add .idea and .vscode into the .gitignore. Thus, adding .DS_Store could make sense to this project.

Not adding these stuff is kinda my personal preference which was suggested by others back to the time I contributed to git-extras. They suggested me adding these config to my local development environment. Otherwise, we'll need to add lots and lots different kinds of temp file and config into .gitignore. Just share the reason why I asked you to remove it at the first place 🙂

@Lee-W Lee-W merged commit ae22794 into commitizen-tools:master Jan 22, 2023
@12rambau 12rambau deleted the color branch April 24, 2023 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants