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

pre-commit autoupdate should not change rev fields like master or stable #1354

Closed
lucatrv opened this issue Mar 6, 2020 · 16 comments
Closed
Labels

Comments

@lucatrv
Copy link

lucatrv commented Mar 6, 2020

I set most of my hooks' rev fields to master, or stable for black, see instructions. If I run pre-commit autoupdate, all rev fields are set explicitly to the latest version number, and I need to revert all them back manually. pre-commit autoupdate should update only those rev fields that are set to an explicit version number.

@asottile
Copy link
Member

asottile commented Mar 6, 2020

black's directions are wrong and will be changed eventually: psf/black#420

@lucatrv
Copy link
Author

lucatrv commented Mar 6, 2020

I'm ashamed I didn't notice that documentation section... But now my issue becomes that every time a hook is updated to a new version, the .pre-commit-config.yaml file is changed, and since I use conventional commits, every time I would need to create a new "build: [...]" commit.
What is the suggested solution to this?

@asottile
Copy link
Member

asottile commented Mar 6, 2020

what is the problem?

@lucatrv
Copy link
Author

lucatrv commented Mar 6, 2020

Well... to pollute my git history. Do you suggest to ignore the .pre-commit-config.yaml file or to check it in?

@asottile
Copy link
Member

asottile commented Mar 6, 2020

wait, what are you polluting?

@lucatrv
Copy link
Author

lucatrv commented Mar 6, 2020

I mean, I configured several hooks. Maybe every other day there could be an update, after a few years I would have hundreds of "build: [...]" commits just for .pre-commit-config.yaml. I wanted to avoid that.

@asottile
Copy link
Member

asottile commented Mar 6, 2020

if you read the docs above, the goals are for reproducibility and repeatability. without checking in the file and picking consistent versions you really have no idea what versions you're running and no way to validate

in other words, the world you're describing your repository will (not can) break without changes to the code in the repository

updating the versions of linters is a conscious decision and has a commit associated with it (instead of mysteriously broken builds due to some upstream changing)

you don't have to do it every day unless you want to (but then that would be your decision) -- the stability without updating is very good

I'd encourage you to read up on why repeatability is a desirable quality in software :)

@lucatrv
Copy link
Author

lucatrv commented Mar 6, 2020

I agree that reproducibility and repeatability is a very important concept when considering dependencies and build tools configuration, but (at least in my case) I'm using pre-commit for linting code, in case a linter is updated maybe I would get some new or different lint message, or even an error when committing, but my code would not really break. Or am I mistaken?

@asottile
Copy link
Member

asottile commented Mar 6, 2020

presumably your build is broken if your linters aren't passing

but also code formatters

@lucatrv
Copy link
Author

lucatrv commented Mar 7, 2020

OK, I found this solution: I installed all required packages locally, and configured all hook repos as local, using the specific hook configuration for each package. For instance for black now I have:

repos:
-   repo: local
    hooks:
    -   id: black
        name: black
        description: "Black: The uncompromising Python code formatter"
        entry: black
        language: python
        language_version: python3
        require_serial: true
        types: [python]

Now all packages are managed outside pre-commit, so the .pre-commit-config.yaml does not change anymore.

Are there any drawbacks with this solution?

@asottile
Copy link
Member

asottile commented Mar 7, 2020

yes that has exactly the same problems, except now you're using the less supported escape hatch

@asottile
Copy link
Member

asottile commented Mar 7, 2020

plus your contributors need to install things manually instead of having it automatically managed

@lucatrv
Copy link
Author

lucatrv commented Mar 7, 2020

I also added all linters to my setup.py file:

setup({
    extras_require={
        'dev': [
            'pre-commit',
            'pre-commit-hooks',
            'isort',
            'black',
            'flake8',
            'pylint',
            ...
        ]
    }
})

@asottile
Copy link
Member

asottile commented Mar 7, 2020

yeah, kind of the whole point of pre-commit is that it manages the installs for you so you don't have some bespoke setup for contributors to try and learn

@lucatrv
Copy link
Author

lucatrv commented Mar 7, 2020

OK, you convinced me, thanks.

chinnichaitanya added a commit to chinnichaitanya/python-discord-logger that referenced this issue Feb 7, 2021
The recent revisions of the pre-commit integration with black has specified to use the version instead of a constant branch name like `stable` so that it could be updated seemlessly using `pre-commit autoupdate`. For reference,

- https://pre-commit.com/#using-the-latest-version-for-a-repository
- pre-commit/pre-commit#1354
- https://github.com/psf/black#version-control-integration
- psf/black#420
chinnichaitanya added a commit to chinnichaitanya/python-slack-logger that referenced this issue Feb 7, 2021
The recent revisions of the pre-commit integration with black has specified to use the version instead of a constant branch name like `stable` so that it could be updated seemlessly using `pre-commit autoupdate`. For reference,

- https://pre-commit.com/#using-the-latest-version-for-a-repository
- pre-commit/pre-commit#1354
- https://github.com/psf/black#version-control-integration
- psf/black#420
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
@asottile @lucatrv and others