feature: Use pre-commit in cookiecutters instead of direct dependencies on black, isort, etc.

[edgarrmondragon]

Feature scope

Other

Description

Advantages

• Fewer headaches around incompatible dependencies by having pre-commit manage a virtualenv for each tool (see [Bug]: Incompatible isort and python version in cookiecutter template #1402)
• Can integrate with pre-commit.ci for autofixes and autoupdates

Disadvantages

• One more tool to manage for developers (e.g. poetry run isort vs pre-commit run isort --all-files

[rafalkrupinski]

Using pre-commit to manage dependencies is a headache. There's no lock file, and since some package use others' internal names, pre-commit often unexpectedly fails when a new patch version changes something.

OCA/oca-addons-repo-template#175 (comment)

[WillDaSilva]

@rafalkrupinski Thank you for bringing pre-commitx and pre-commit.nix to my attention. I love what Nix is doing, and this seems like a good way to manage dev tools.

There is a significant cost to using the same lock file for the library dependencies as we use for all of the linter & dev tool dependencies. It results in all of these tools imposing version requirements that affect the entire library. This can make upgrading dependencies difficult or impossible.

As for using Nix for tools managed by pre-commit, I'll echo what Carmen Bianca said:

I like the idea a lot, and I'm very charmed by Nix, but I don't think this is presently a workable solution due to the aforementioned learning curve. I wish Nix were easier than it is.

Just learning enough Nix to get the linters to run wouldn't be exactly sufficient, either. Inevitably, the user will want to reproduce some error that appears in the linter but not in their own Python virtualenv, and they'll need enough know-how of Nix to navigate that.

In practice I haven't found pre-commit to cause issues of this sort, presumably since we test the updates to our pre-commit managed tools in the pre-commit.ci auto-update PRs. While installing each tool into a virtual environment on the host system doesn't provide the practically-perfect dependency management offered by Nix, it works well enough in practice, and is a step up from installing all of the tools into the same virtual environment as would typically be done if we used poetry.lock to control their versions instead of pre-commit.

[rafalkrupinski]

Ah, sorry, I didn't mean to promote nix, I find it to heavy too. I only meant to point out problems that pop-out unexpectedly with pre-commit, and I just happen to know that link :)
Glad you have it worked out

[rafalkrupinski]

How about nox?

[WillDaSilva]

@rafalkrupinski nox is more of a replacement for tox than pre-commit. Additionally, it doesn't have/use a lockfile, which was your original issue with this suggestion to use pre-commit as the source of truth for linting dependencies.

[edgarrmondragon]

I agree with @WillDaSilva. Reproducibility and stability are desired properties for any workload, but I think we can tolerate a slight lack of both for linters in favor of avoiding dependency conflicts with actual package dependencies.

Closed by #1648