Use require_serial for sphinx-lint in .pre-commit-config.yaml

[ezio-melotti]

Pull Request

Pull Request Checklist

• Read and followed this repo's Contributing Guidelines
• Based your PR on the latest version of the correct branch (master or 4.x)
• Checked your writing carefully for correct English spelling, grammar, etc
• Described your changes and the motivation for them below
• Noted what issue(s) this pull request resolves, creating one if needed

Description of Changes

sphinx-lint already uses multiprocessing internally, and this might interfere with precommit, since it will try to use multiple processes too. By setting require_serial we prevent precommit from doing that.

See:

• Use require_serial: true as the default for our pre-commit hook, not -j1 sphinx-contrib/sphinx-lint#95

[CAM-Gerlach]

I'm a little confused—wasn't the purpose of including this in the pre-commit-hooks config in sphinx-contrib/sphinx-lint#95 , released in Sphinx-Lint 0.8.1 which we adopted in #351 , such that each individual project wouldn't need to add this?

Testing locally with time pre-commit run --all sphinx-lint on an old, slow single-threaded by decent multi-threaded with pre-commit 2.20.0 on both master and this branch that are identical except for this setting, I get essentially identical times for running this check, well within the margin of error (3.75 s +/- 0.3 s vs 3.87 s, +/- 0.4 s across 5 trials each), which suggests it has no measurable effect.

Is there something I'm missing here? Thanks!

[ezio-melotti]

Is there something I'm missing here?

Actually I was the one who got confused.

To summarize:

• both pre-commit and sphinx-lint use multiple processes
• if they both use them at the same time, they interfere with each other, so one of the two should be disabled
• for sphinx-lint, this can be achieved by specifying --jobs=1, for pre-commit there is require_serial
• both solutions seem to have similar performances on average
• by specifying require_serial in .pre-commit-hooks.yaml in the sphinx-lint repo, we are already telling pre-commit not to use multiple processes, and in doing so, we prevent conflicts with projects that explicitly sets a specific number of jobs for sphinx-lint

IOW, this PR is not necessary, and can be closed. Sorry for the noise!

[CAM-Gerlach]

Thanks Ezio!