-
-
Notifications
You must be signed in to change notification settings - Fork 287
refactor(questions): type questions with TypedDict #1485
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: refactors
Are you sure you want to change the base?
refactor(questions): type questions with TypedDict #1485
Conversation
@@ -52,7 +52,7 @@ def prompt_commit_questions(self) -> str: | |||
# Prompt user for the commit message | |||
cz = self.cz | |||
questions = cz.questions() | |||
for question in filter(lambda q: q["type"] == "list", questions): | |||
for question in (q for q in questions if q["type"] == "list"): |
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.
type inference only works this way
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## refactors #1485 +/- ##
=============================================
+ Coverage 97.87% 97.88% +0.01%
=============================================
Files 57 58 +1
Lines 2677 2700 +23
=============================================
+ Hits 2620 2643 +23
Misses 57 57
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:
|
@@ -9,7 +9,7 @@ | |||
|
|||
from commitizen import git | |||
from commitizen.config.base_config import BaseConfig | |||
from commitizen.defaults import Questions | |||
from commitizen.question import CzQuestion |
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 might be able to move it to type_checking. but this can be next pr or let ruff handle it in the future
# Type | ||
Questions = Iterable[MutableMapping[str, Any]] | ||
Questions = Iterable[MutableMapping[str, Any]] # TODO: deprecate this? |
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.
sounds good
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.
Mostly good; respond to one of your questions. Let me know if you want to address it in this PR or the next. Thanks!
baee83e
to
a9cd957
Compare
0a2c7b3
to
f18342b
Compare
Description
A better version of #1467
Make the type of questions more strict. Not sure whether we should deprecate the type
Questions
.Checklist
Code Changes
poetry all
locally to ensure this change passes linter check and testsDocumentation Changes
poetry doc
locally to ensure the documentation pages renders correctlyExpected Behavior
Steps to Test This Pull Request
Additional Context