-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update pre-commit
to use pinned version of dvc
and correct stage names
#10621
base: main
Are you sure you want to change the base?
Conversation
`pre-commit` [changed the acceptable stage names](https://pre-commit.com/#confining-hooks-to-run-at-certain-stages)
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## main #10621 +/- ##
==========================================
- Coverage 90.68% 90.67% -0.01%
==========================================
Files 504 504
Lines 39795 39796 +1
Branches 3141 3141
==========================================
- Hits 36087 36085 -2
- Misses 3042 3045 +3
Partials 666 666 β View full report in Codecov by Sentry. |
thanks @mkdjr ! Is it still in draft, needs more testing? |
Oh yeah, I forgot to take it out of its drafted state. I think it's good to go, but should probably be followed up with a future PR to automate updating the |
@@ -16,19 +16,19 @@ def pre_commit_install(scm: "Git") -> None: | |||
with modify_yaml(config_path) as config: | |||
entry = { | |||
"repo": "https://github.com/iterative/dvc", | |||
"rev": "main", | |||
"rev": "3.56.0", |
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.
Let's not hardcode version. That will break our release workflow.
"rev": "3.56.0", | |
"rev": ".".join(map(str, version_tuple[:3])), |
You can import this from from dvc import version_tuple
.
β I have followed the Contributing to DVC checklist.
π If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
1. Pin
rev
argument inpre-commit-config.yaml
writerThe
pre-commit
package recommends against using mutable references such asmain
, as the version of the package pulled in the hook (in this casedvc
), does not get updated after the first timepre-commit
installs it. This PR addresses this by instead pinning the hooks todvc
v3.56.0. This change is made with the goal of:pre-commit
warning, anddvc
version thatpre-commit
refers to whendvc
receives a new release.As a reference,
ruff
, the python linter and formatter, keeps its pre-commit hooks in a separate repository. This repository has its own pre-commit hooks that automatically pull the latest version ofruff
and updates the version pointer in thepre-commit-hooks.yaml
file. Maybedvc
could do something like this in the future to automate thepre-commit-hooks.yaml
update process.2. Update
stages
keywordsAdditionally,
pre-commit
has now deprecated the acceptable values for thestages
keyword forhooks
. I have updated thedvc install
command and thepre-commit-hooks.yaml
file to reflect this change.As this change was made in
pre-commit
v3.2.0, thedvc
hooks now requirepre-commit
to be updated to at least v3.2.0.Closes #9897.
Associated documentation PR: iterative/dvc.org#5323.