-
-
Notifications
You must be signed in to change notification settings - Fork 280
cli(script): gen screenshot for cli helps #1048
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
cli(script): gen screenshot for cli helps #1048
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1048 +/- ##
==========================================
+ Coverage 97.33% 97.48% +0.15%
==========================================
Files 42 55 +13
Lines 2104 2429 +325
==========================================
+ Hits 2048 2368 +320
- Misses 56 61 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This looks super cool, do you think it's possible to generate animated gifs as well? |
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.
Hi @Ellie-Yen , thank you for your immediate PR! This looks promising! But there are a few things I will need your help with before we can merge it.
- address the comments we have in separate files
- reword the commit messages into
ci
instead offeat
as they're not commitizen features but more like a documentation improvement
After we merge this one, there are a few steps we want to do
- Add these images to the document (somewhere like https://commitizen-tools.github.io/commitizen/init/)
- Automate the update images scripts in github actions
Thanks again for this cool script
57790bd
to
c1c2711
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.
Thank you for the prompt update! We're really close to merging.
scripts/gen_cli_help_screenshots.py
Outdated
"""Generate the screenshot for help message on each cli command and save them as svg files.""" | ||
if not os.path.exists(images_root): | ||
os.makedirs(images_root) | ||
print(f'created {images_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.
print(f'created {images_root}') | |
print(f"Created {images_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.
we can run ruff or black against these files to improve style consistency
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.
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.
yep, I think that what we should do
c1c2711
to
6fd16eb
Compare
6fd16eb
to
cba2c06
Compare
@Ellie-Yen Great job! I think we're good to merge this one, but I'll keep it open for a few days so that others can take a look. btw are you interested in finishing these @woile @noirbizarre I'm planning on merging this one these days. Let me know if you have any thoughts. Thanks! |
We could make it a future improvement 👍 |
Thanks for reviewing ~ |
Description
Add a script to generate screenshot of all cli help contents.
Checklist
./scripts/format
and./scripts/test
locally to ensure this change passes linter check and testExpected behavior
Execute
scripts/gen_cli_help_screenshots.py
will generate screenshots of cli help instructionsSteps to Test This Pull Request
scripts/gen_cli_help_screenshots.py
commitizen.cli
'sdata
is match its screenshot indocs/images/cli_help/
folder.Additional context
Considering the purpose of the file is more closed to a script, put it into scripts folder.
Should add test and update the documentation accordinately after the change is accepted.