Skip to content
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 version in sample config #1611

Merged
merged 1 commit into from Sep 27, 2020
Merged

Update pre-commit version in sample config #1611

merged 1 commit into from Sep 27, 2020

Conversation

mcsitter
Copy link
Contributor

@mcsitter mcsitter commented Sep 23, 2020

This is a minor change. The sample config doesn't even have the latest major version in it.

This provides the solution to what is proposed in the TODO.
The output from

git -c 'versionsort.suffix=-' ls-remote --exit-code --refs --sort='version:refname' --tags git://github.com/pre-commit/pre-commit-hooks '*.*.*' | tail --lines=1 | cut --delimiter='/' --fields=3

can be directly inserted inplace of the defined rev.

Alternatively one could borrow parts from the autoupdate command. The logic there seems to init a repository and then parses the output from git commands using functions from the util module. The above may or may not be overkill. I have not looked into the details of the different approaches yet. Seems doable though!

If that change is desired, I would be happy to update the PR accordingly.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile
Copy link
Member

there isn't a way to do this fast and correct, I'll probably just delete the TODO

notably:

  • your oneliner does not account for forward history, just an assumption by sorting
  • the autoupdate is correct, but is far too slow due to needing a full clone

@asottile asottile merged commit 949ea16 into pre-commit:master Sep 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants