Skip to content

Conversation

@12rambau
Copy link
Contributor

Description

Since pre-commit 2.18.0, it is possible to tell pre-commit which hook to install directly from the .pre-commit-config.yaml file. Thus I update the said file to apply the hooks according to the docs: pre-commit install -t pre-commit -t pre-push -t commit-msg. I also updated the pre-commit version to make sure this parameter is available and finally updated the contributing documentation to make sure people simply use the pre-commit installcommand.

bonus: there was a typo that I corrected (it's "stageS")

Checklist

  • 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 change

@12rambau12rambau changed the title Pre commitrefactor: set default_install_hook_typesApr 5, 2023
@12rambau12rambau marked this pull request as ready for review April 5, 2023 15:49
@12rambau
Copy link
ContributorAuthor

it needs #607 to be solved first as this requires at least 3.7

@woile
Copy link
Member

woile commented Apr 6, 2023

Thanks for the PR! Could you point this PR to v3 branch? because of the 3.7 dep

@12rambau12rambau changed the base branch from master to v3April 7, 2023 07:41
@12rambau
Copy link
ContributorAuthor

Thanks for the PR! Could you point this PR to v3 branch? because of the 3.7 dep

done

@codecov
Copy link

codecovbot commented Apr 7, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 🎉

Comparison is base (1a1e73b) 97.37% compared to head (1153908) 97.42%.

Additional details and impacted files
@@ Coverage Diff @@## master #697 +/- ## ========================================== + Coverage 97.37% 97.42% +0.04%  ========================================== Files 42 42 Lines 2022 2022 ========================================== + Hits 1969 1970 +1 + Misses 53 52 -1 
FlagCoverage Δ
unittests97.42% <100.00%> (+0.04%)⬆️

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

Impacted FilesCoverage Δ
commitizen/__version__.py100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@woile
Copy link
Member

Would you mind rebasing on top of v3? Thanks!

@12rambau
Copy link
ContributorAuthor

isn't it already the case ?

@woile
Copy link
Member

woile commented Apr 19, 2023

No, v3 was merged, you can see all kind of unrelated commits in the list.

You can follow this instructions to rebase: #686 (comment)

@12rambau
Copy link
ContributorAuthor

12rambau commented Apr 19, 2023

sorry I'm used to work in repositories where we squash merge PR so this is not considered as an issue.

@12rambau
Copy link
ContributorAuthor

not sure I did the right thing but THB it's a super small modifcation. merge squash it it wil make everyone's life easier.

Also the PR will look strange but upon merging, git resolves the duplicates commits (as they have the same hash) so why rebasing ?

@woilewoile requested a review from Lee-WApril 23, 2023 05:23
@Lee-WLee-W changed the base branch from v3 to masterApril 23, 2023 06:24
@Lee-WLee-W added the pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check label Apr 23, 2023
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.

Thanks @12rambau . This looks great!

@woile I'm planning on merging this one today.

@woilewoile merged commit bcddd8d into commitizen-tools:masterApr 23, 2023
@12rambau12rambau deleted the pre-commit branch April 23, 2023 09:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-status: ready-to-mergealmost ready to merge. just keep it for a few days for others to check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@12rambau@woile@Lee-W