Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(commit): implement questions 'filter' support with handlers#1207

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
AdrianDC wants to merge7 commits intocommitizen-tools:master
base:master
Choose a base branch
Loading
fromAdrianDC:questions-filter

Conversation

@AdrianDC
Copy link
Contributor

Description

Supported APIs: Common Python, commitizen.cz.utils.* functions

Example YAML configurations:

commitizen:name:cz_customizecustomize:questions:      -...      -type:inputname:scopemessage:'Scope of the change :'filter:'lambda text: commitizen.cz.utils.required_validator(text, msg="! Error: Scope is required")'default:''      -type:inputname:subjectmessage:'Title of the commit (starting in lower case and without period) :'filter:'lambda text: commitizen.cz.utils.required_validator(text.strip(".").strip(), msg="! Error: Title is required")'default:''      -type:inputname:bodymessage:'Additional contextual message (Empty to skip) :'default:'Issue: #...'filter:'commitizen.cz.utils.multiple_line_breaker'

Additional commits to add missing YAML tests, fix YAML configurations, and then add the keys for this feature.

Checklist

  • Add test cases to all the changes you introduce
  • Run./scripts/format and./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

? Title of the commit (starting in lower case and without period) :                                                                                                                                                                                    Hello                                                                                                                                                                                                                                                  ! Error: Title is required

Steps to Test This Pull Request

Example provided in the description

Additional context

@codecov
Copy link

codecovbot commentedAug 13, 2024•
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.26%. Comparing base (120d514) to head (e01b323).
⚠️ Report is 805 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@##           master    #1207      +/-   ##==========================================+ Coverage   97.33%   98.26%   +0.92%==========================================  Files          42       58      +16       Lines        2104     2712     +608     ==========================================+ Hits         2048     2665     +617+ Misses         56       47       -9
FlagCoverage Δ
unittests98.26% <100.00%> (+0.92%)⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AdrianDC
Copy link
ContributorAuthor

  • Coverage resolved, 100% over commit.py
  • TOML faulty types fixed
  • Local containerized tests fixed for 'safe.directory' ownership issues
  • Tests for 'cz_customize' using the.customize config rather than unrelatedcz_jira
  • Contents of the questions prompts tested for coverage and types aroundcommands.Commit

@AdrianDCAdrianDCforce-pushed thequestions-filter branch 2 times, most recently from780b16b toc24db8aCompareAugust 18, 2024 13:38
Copy link
Member

@Lee-WLee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm sending a "Request changes" to this PR as I'm -1 to this but would like to get feedback from other maintainers

@AdrianDCAdrianDC changed the titlefeat(commit): implement questions 'filter' support with evaluationsfeat(commit): implement questions 'filter' support with handlersAug 25, 2024
@AdrianDCAdrianDCforce-pushed thequestions-filter branch 2 times, most recently from65f4d93 to314f3bfCompareAugust 25, 2024 03:38
@AdrianDC
Copy link
ContributorAuthor

Refactored as discussed :feat(commit): implement questions 'filter' support with handlers

Supported APIs:

  • multiple_line_breaker
  • required_validator
  • required_validator_scope
  • required_validator_subject_strip
  • required_validator_title_strip

Example YAML configurations:

commitizen:name:cz_customizecustomize:questions:      -...      -type:inputname:scopemessage:'Scope of the change :'filter:'required_validator_scope'default:''      -type:inputname:subjectmessage:'Title of the commit (starting in lower case and without period) :'filter:'required_validator_subject_strip'default:''      -type:inputname:bodymessage:'Additional contextual message (Empty to skip) :'default:'Issue: #...'filter:'multiple_line_breaker'

@AdrianDCAdrianDCforce-pushed thequestions-filter branch 2 times, most recently from8e411f3 to33bbf0fCompareNovember 16, 2024 15:13
@Lee-WLee-W self-requested a reviewNovember 18, 2024 14:44
@AdrianDC
Copy link
ContributorAuthor

Rebased 😉 .

Please cherry-pick at least the coverage improvements and minor fixes before the filter features review 👍 .

Copy link
Member

@Lee-WLee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Sorry for taking so long. Just left some comments

@Lee-W
Copy link
Member

I'll come back to this one after 4.8.3 release. As for another PR, I'll take a look at these 2 days. Sorry for the long waiting and thanks so much for still being around!

AdrianDC reacted with thumbs up emoji

Signed-off-by: Adrian DC <radian.dc@gmail.com>
> commitizen.exceptions.InvalidConfigurationError: Failed to parse not_exist.yaml: while scanning a double-quoted scalar> found unknown escape character 's'Signed-off-by: Adrian DC <radian.dc@gmail.com>
> test_commit_parser[config2] - AssertionError:>   assert '(?P<message>.*)' == '^(?P<change_...<message>.*)?'> test_changelog_pattern[config2] - AssertionError:>   assert '.*' == '^(feature|bug fix)?(!)?'> test_change_type_map[config2] - AssertionError:>   assert None == {'bug fix': 'Fix', 'feature': 'Feat'}Signed-off-by: Adrian DC <radian.dc@gmail.com>
Details: If using a TOML configuration, the type was 'tomlkit.items.AoT'---Signed-off-by: Adrian DC <radian.dc@gmail.com>
Signed-off-by: Adrian DC <radian.dc@gmail.com>
Supported APIs:  - multiple_line_breaker  - required_validator  - required_validator_scope  - required_validator_subject_strip  - required_validator_title_stripExample YAML configurations:---commitizen:  name: cz_customize  customize:    questions:      - ...      - type: input        name: scope        message: 'Scope of the change :'        filter: 'required_validator_scope'        default: ''      - type: input        name: subject        message: 'Title of the commit (starting in lower case and without period) :'        filter: 'required_validator_subject_strip'        default: ''      - type: input        name: body        message: 'Additional contextual message (Empty to skip) :'        default: 'Issue: #...'        filter: 'multiple_line_breaker'---Signed-off-by: Adrian DC <radian.dc@gmail.com>
@AdrianDC
Copy link
ContributorAuthor

Rebased from last year's PR, to update towards4.9.1+adriandc.20250914
forpre-commit-crocodile 8.0.0 (prek becoming default btw).

Feel free to edit the PR or cherry-pick at will.

I also implemented acheck-commitizen-branch wrapper to thecommitizen-branch hook,
handlingset-head if needed, covering#704, also#593, but fixing upstream needs analysis.

For the record, I'm not a fan of pinning to a patched & customized fork for so long,
blocking my users + coworkers on a specific version of commitizen-tools that I rebase 2-3 times a year,
including now v5-moved fixes and questions features, but works fine since August 2024.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@woilewoileAwaiting requested review from woilewoile is a code owner

@noirbizarrenoirbizarreAwaiting requested review from noirbizarrenoirbizarre is a code owner

@Lee-WLee-WAwaiting requested review from Lee-WLee-W is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@Lee-WLee-W

Projects

None yet

Milestone

4.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@AdrianDC@Lee-W@noirbizarre@woile
[8]ページ先頭
©2009-2025 Movatter.jp