Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 305
refactor(customize): improve code readability#1412
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
refactor(customize): improve code readability #1412
Uh oh!
There was an error while loading. Please reload this page.
Conversation
bearomorphism commented May 14, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
316579c to 39792a0Comparecodecovbot commented May 14, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## master #1412 +/- ## ========================================== + Coverage 97.33% 97.59% +0.25% ========================================== Files 42 57 +15 Lines 2104 2657 +553 ========================================== + Hits 2048 2593 +545 - Misses 56 64 +8
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:
|
39792a0 to ce36db5Comparebearomorphism commented May 15, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I was curious about the difference between https://chatgpt.com/share/68254494-f83c-8010-8ab3-7f8808682bcf probably not important |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
noirbizarre commented May 15, 2025
Actually, it matters: if you have
So it really depends on your case, but if there is no default value, and you want to be sure of the type you have in case of falsy value, go for the 2nd. |
ce36db5 to e163049Comparebearomorphism commented May 15, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Thank you for explanation. Actually, the thing I'm not sure is if we want to fallback to empty string if the value is falsy (current implementation). |
noirbizarre commented May 15, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I think this has been a recurrent discussion on reviews: we have some improvements to do on the "cli flags > user settings > default value" cascading handling:
There is room for improvement. |
Lee-W commented May 16, 2025
I've also wanted to rewrite the whole setting, default value thing.... |
bearomorphism commented May 20, 2025
I can try this. We can discuss when you have time. |
Lee-W commented May 22, 2025
Created an issue #1445 to track and discuss this redesign, probably will go to v5 |
d0c2674 into commitizen-tools:masterUh oh!
There was an error while loading. Please reload this page.
Description
Make the code shorter but still easy to read.
Checklist
poetry alllocally to ensure this change passes linter check and testExpected behavior
NA, this is refactor.
Steps to Test This Pull Request
Additional context
NA