-
-
Notifications
You must be signed in to change notification settings - Fork 281
docs: improve clarity of docs and fix typo/grammar issues #1404
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1404 +/- ##
==========================================
+ Coverage 97.33% 97.57% +0.23%
==========================================
Files 42 57 +15
Lines 2104 2680 +576
==========================================
+ Hits 2048 2615 +567
- Misses 56 65 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 !
Niiiice! @bearomorphism but please use |
@Lee-W But there's also a small behavioral change in |
I probably wouldn't consider it as a fix that we need to bump a version 🤔 |
Ok let me rebase it, thanks |
6119504
to
4c1d1a1
Compare
Let me also fix #1405 in this PR to avoid catastrophic conflicts :) |
@noirbizarre @Lee-W I did a lot of changes in README after your latest reviews. Please kindly review my changes again when you have a moment. Thanks! |
148b9b9
to
41fbfe2
Compare
4b7a7cd
to
2cc458e
Compare
I also rewrote |
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.
I'm good with what we currently have in this PR. The docs look so nice. But it would be awesome if we could have some of the minor nit-picks addressed 🙂
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. I'll keep it open for a few days so others can take a look as well. (same for other docs PR) I'm planning on merging these this weekend
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.
That's quite some work you did there, thanks!
Here is some suggested changes
"For more information about the topic go to " | ||
"https://conventionalcommits.org/" | ||
"Commitizen is a powerful release management tool that helps teams maintain consistent and meaningful commit messages while automating version management.\n" | ||
"For more information, please visit https://commitizen-tools.github.io/commitizen" |
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.
We definitively need to move the documentation to the root!
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.
**(For macOS users) Using Homebrew:** | ||
```bash | ||
brew install commitizen | ||
``` |
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.
Not sure whether we should provides OS/distro specific installation.
If we start doing this, we should do it for all, not just macOS/Homebrew:
- ArchLinux: https://aur.archlinux.org/packages/python-commitizen
- Ubuntu: https://launchpad.net/ubuntu/+source/commitizen
- Debian: https://packages.debian.org/sid/commitizen
- ...
Maybe instead we should have a more generic paragraph saying that some OS package commitizen
but we are not officially maintaining those.
**Using Poetry:** | ||
```bash | ||
# For Poetry >= 1.2.0 | ||
poetry add commitizen --group dev | ||
|
||
# For Poetry < 1.2.0 | ||
poetry add commitizen --dev |
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.
Same reasoning, maybe we should add at least uv
and pdm
You can customize: | ||
- [Version files](./commands/bump.md#version_files) | ||
- [Version scheme](./commands/bump.md#version_scheme) | ||
- [Version provider](./config.md#version-providers) | ||
|
||
For all available options, see the [bump command documentation](./commands/bump.md). |
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.
From the readme, we should link to the documentation site, not the markdown files themselves
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.
I'm not sure about this one. I think this might affect the page building 🤔
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.
probably not, but it would be a problem if we move the doc to root
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.
I'm not sure. If we link to the documentation site, there will be no warnings of URL typos such as
INFO - Doc file 'config.md' contains a link 'commands/bump.md#-post_bump_hooks', but the doc 'commands/bump.md' does not
contain an anchor '#-post_bump_hooks'.
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.
I prefer to keep it as is to prevent broken links.
- [Exit Codes Reference](./exit_codes.md) | ||
- [Configuration Guide](./config.md) | ||
- [Command Documentation](./commands/init.md) |
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.
Same, link to the doc, not the markdown files
Closes #1405, #1413
Description
When I first started looking into how to use this tool, I found the documentation really hard to read.
It's such a pity for such a useful tool!
Thanks to powerful AI tools, improving clarity of the documents can be easily done.
Checklist
poetry all
locally to ensure this change passes linter check and testExpected behavior
It should be easier for the readers to understand what Commitizen does after they enter the landing page of Commitizen.
The typo of the cli output should be fixed.
Steps to Test This Pull Request
NA
Additional context
NA