Skip to content

not enforcing python3.6 for precommit hook#430

Merged
ambv merged 1 commit intopsf:masterfrom
miki725:patch-1
Aug 17, 2018
Merged

not enforcing python3.6 for precommit hook#430
ambv merged 1 commit intopsf:masterfrom
miki725:patch-1

Conversation

@miki725
Copy link
Contributor

@miki725 miki725 commented Jul 31, 2018

this should allow precommit hooks to be used with py37

this should allow precommit hooks to be used with py37
@coveralls
Copy link

coveralls commented Jul 31, 2018

Pull Request Test Coverage Report for Build 599

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.003%

Totals Coverage Status
Change from base Build 590: 0.0%
Covered Lines: 2618
Relevant Lines: 2727

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 599

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.003%

Totals Coverage Status
Change from base Build 590: 0.0%
Covered Lines: 2618
Relevant Lines: 2727

💛 - Coveralls

Copy link
Contributor

@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.

Either value here is fine.

This is overridable in the local .pre-commit-config.yaml:

-   repo: https://github.com/ambv/black
    rev: ...  # fill this out!
    hooks:
    -   id: black
        language_version: python3  # or python3.7 or whatever you like!

@ambv
Copy link
Collaborator

ambv commented Aug 17, 2018

I understand that the problem is that we should be using Python 3.7 instead. However, the change as is will inevitably end with somebody getting Python 3.5 and then complaining here. Anthony, any way we can specify minimum version?

@miki725
Copy link
Contributor Author

miki725 commented Aug 17, 2018

dont think you can specify ranges in pre-commit itself - https://pre-commit.com/#overriding-language-version

black itself though enforces python version in setup.py via python_requires . If someone will attempt to use precommit with older python, black will fail to install. Dont have older py3 version installed so cant test for certain.

https://github.com/ambv/black/blob/3fb451687293c993b10d860449eb405ddd44f377/setup.py#L42

@asottile
Copy link
Contributor

Not currently, pre-commit itself doesn't actually know or care about the values of language_version and just passes them through to the underlying tool (virtualenv / nodeenv / rbenv / etc.).

tbh, language_verison: python3 and then docs about requiring python3.6 and how to set that probably makes the most sense. At least for macos we've had to override this in our repositories to language_verison: python3

@ambv
Copy link
Collaborator

ambv commented Aug 17, 2018

@miki725 is right, Black will just fail to install on an old Python 3 so we're good anyway. Let's have this.

@ambv ambv merged commit 33601ff into psf:master Aug 17, 2018
@ambv
Copy link
Collaborator

ambv commented Aug 17, 2018

Thanks! ✨ 🍰 ✨

@miki725 miki725 deleted the patch-1 branch August 17, 2018 16:07
@miki725
Copy link
Contributor Author

miki725 commented Aug 17, 2018

no problem. glad to help, at least in some small way. and thanks for merging!

btw the project is awesome!!! thanks for sharing it with everyone

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants