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

Do version numbers have to have a '.'? #1794

Closed
paddyroddy opened this issue Feb 9, 2021 · 7 comments
Closed

Do version numbers have to have a '.'? #1794

paddyroddy opened this issue Feb 9, 2021 · 7 comments
Labels

Comments

@paddyroddy
Copy link

Looking at this line it looks to be testing for the presence of a dot in the version number

if '.' not in rev and not re.match(r'^[a-fA-F0-9]+$', rev):
but what if a repo is just following a simple v1/v2 model?

I'm using this repo https://gitlab.salort.eu/jsalort/latexhook/-/tags which has a v1 tag. My .pre-commit-config.yaml looks like this:

repos:
    - repo: https://gitlab.salort.eu/jsalort/latexhook
      rev: v1
      hooks:
          - id: latexindent

This means every time i commit I get this error at the top
image

My version is pre-commit==2.10.1 and I don't think this used to happen

@paulhfischer
Copy link
Contributor

thanks for the issue, you're correct this warning is new in pre-commit 2.10 (#1715) but shouldn't be triggered in your case (it's there to warn when you are pointing to master or HEAD in .pre-commit-config.yaml).

for now (until the detection is improved/updated) you could use the (short) sha the tag points to, in your case 7879fc02.

@paddyroddy
Copy link
Author

okay cheers, that does that job

@asottile
Copy link
Member

asottile commented Feb 9, 2021

yeah unfortunately due to moving tags we're using a string validation for whether something looks like a mutable reference.

the other unfortunate bit is I've seen v1 / v2 be rather common moving tags so I'm not sure if this will get fixed -- if the tag becomes v1.0 it should pass the warning check

the warning is mostly intended to catch conventionally floating things like master / stable / etc.

@paddyroddy
Copy link
Author

paddyroddy commented Feb 9, 2021

Could it be changed to something like

if '.' not in rev and not re.match(r'^[a-fA-F0-9]+$', rev) and not re.match('^v[0-9]+$', rev):

Thus having what you have now but also allowing i.e. v1.

@asottile
Copy link
Member

asottile commented Feb 9, 2021

a change like that could work but:

the other unfortunate bit is I've seen v1 / v2 be rather common moving tags

so I'm not sure if that would be a good idea.

@paddyroddy
Copy link
Author

ah okay, sorry didn't really know what you meant by that

@asottile
Copy link
Member

asottile commented Apr 4, 2021

yeah so for now I'm going to answer this issue as "yes" -- we may revisit if this comes up more frequently

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

No branches or pull requests

3 participants