pre-commit sample configuration should not suggest `rev: stable`

[asottile]

👋 hello, pre-commit maintainer here :)

The sample configuration currently suggests rev: stable for black's repository setup. A mutable ref poses some problems. Notably it gives the illusion that the latest version is being used but in reality it uses the version at install time.

This setup isn't suitable for those looking for a repeatable and reproducible linting experience.

The suggestion that we use for our official repositories is to either list the version explicitly in the docs (though this adds tedium to the release cycle) or to use some suitable non-value with instructions on how to substitute an appropriate value.

Yet another option would be to suggest pre-commit autoupdate after writing such a configuration as this will usually convert the mutable rev to an immutable tag (or sha if --bleeding-edge is passed).

[ambv]

Thanks for reaching out! Sorry for staying quiet, I was happily crousing through flyover states when you wrote this.

I'm intrigued by the autoupdate option, I didn't really think much about the moving tag not actually updating on people's pre-commit installations. This is a problem that we need to address.

[asottile]

No problem! travel is good!

I can take a stab at this if you'd like, depends on what option you'd like to call out in the docs -- I can help with a PR if you'd like too!

[ambv]

Would be great if you fixed that for us, sure :-) First, I'd like to understand what you propose we should do.

[asottile]

I don't want to add more release burden for you so the "list the exact version and bump it as released" seems probably out of the question. Using a suitable non-value is straightforward, but leads to a huge amount of SEO for this issue for some reason.

Probably the best solution is to keep the current sample configuration but then right before the "pre-commit install" suggest something along the lines of:

To get a repeatable installation make sure to change the moving tag `stable` to a release of
`black`.  The easiest way to do this is by running `pre-commit autoupdate`
(or `pre-commit autoupdate --repo https://github.com/ambv/black`).

[zsol]

@asottile have you considered solving this from pre-commit itself? That seems a better place to solve this, as I imagine any project providing a precommit config will run into this problem.

Maybe the commit hook could check if there's a new version and print a warning/offer to upgrade? Hey, your hooks passed but you have 3 plugins that are outdated. Here's how to update them: ... Here's how to disable this message: ...

[asottile]

I've hesitated because it adds two things I don't want:

• slowness
• phone home

[ambv]

What Zsolt is suggesting could be opt in (and I would advertise this in my README) and could happen asynchronously while hooks are running.

[asottile]

yeah I'm not saying I'm opposed to adding something like it, but it's not complexity I've decided to take on yet (there's also the whole question of making async work gracefully in python2, exposing errors from it, preempting if hooks finish faster, not taking resources, adding an option to opt into it, documenting it, messaging for it, and a whole slew of other considerations). It's not a simple feature

[jmknoble]

asottile:

Yet another option would be to suggest pre-commit autoupdate after writing such a configuration as this will usually convert the mutable rev to an immutable tag (or sha if --bleeding-edge is passed).

I'm in favor of that. If someone's adding a hook for use with pre-commit, they should pretty much autoupdate anyway.

You could even, if you wish, recommend:

pre-commit autoupdate --repo https://github.com/ambv/black

as that would have localized effect (for black only).

[beenje]

Just ran in this issue. I was using "stable" but my local version of black and the one on my ci was different making my test to fail.

The pre-commit documentation clearly states that mutable rev are not supported:

pre-commit assumes that the value of rev is an immutable ref (such as a tag or SHA) and will cache based on that. Using a branch name (or HEAD) for the value of rev is not supported and will only represent the state of that mutable ref at the time of hook installation (and will NOT update automatically).

Reading this issue 974, it doesn't look like it's gonna change. Would be nice to have black documentation in sync.