-
-
Notifications
You must be signed in to change notification settings - Fork 301
Refactor config #457
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 config #457
Conversation
@woile this branch will need to wait for #456 to be merged. Although this is ready to be reviewed, I'm thinking of adding the functionality of validating config (#300) and overriding customize config in base config (related to #264 (comment)). do you think I should finish all those features then send them to you? or do you want me to create separate PRs for those functionalities?
This looks good, I would go one step at a time.
I would definitely update the documentation, specially to mention the new data types when building a custom commitizen ruler.
Using questions(...) -> list: is quite intuitive because everyone knows what a list is. If using Questions it should be documented and explained that it is basically the same as a list, what do you think?
One of the goals of this and the following PRs is to remove Generic type annotation and go for specific ones. So I would prefer to use either Question or Iterable[MutableMapping[str, Any]]. I'll add the documentation to address it these days. Will change this branch from draft to ready when I've done so.
...efaults to ConventionalCommitsCz These defaults are not defaults for other cz
ef3fb95 to
9ab3912
Compare
Codecov Report
@@ Coverage Diff @@ ## master #457 +/- ## ========================================== + Coverage 97.92% 97.96% +0.04% ========================================== Files 39 39 Lines 1395 1424 +29 ========================================== + Hits 1366 1395 +29 Misses 29 29
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Sounds good to me 💪🏻 looking forward
I've updated the content in docs/customization.md Could you please check whether I need to add it to elsewhere? Thanks!
I think is good. The commit has a typo only 😅
Do you mean the missing s after expect? If so, I've fixed that. If not so, could please you point it out 😱
As this is approved, and I tried to address the typo. I plan to merge this one late today
Uh oh!
There was an error while loading. Please reload this page.
Description
Checklist
./script/formatand./script/testlocally to ensure this change passes linter check and testExpected behavior
no new features has been added. all the functionality should be the same as it used to be
Steps to Test This Pull Request
Additional context
#424, #153