keewis / blackdoc

run black on documentation code snippets
https://blackdoc.readthedocs.io
MIT License
47 stars 4 forks source link

Is it possible to explicitly set black version in pre-commit? #122

Closed max-sixty closed 2 years ago

max-sixty commented 2 years ago

Quoting https://github.com/pydata/xarray/pull/6290#issuecomment-1046291891

FYI something that's a bit surprising is that blackdoc passes in pre-commit locally but not in pre-commit.ci. My guess is that it doesn't specify the version of black it uses and so on my local machine it has an older version of black. I'm not sure whether that's possible with additional_dependencies...

Again — thank you for the excellent tool!

keewis commented 2 years ago

this should definitely be possible, but I'm not quite sure where to put it. I would probably suggest to do that in the project's .pre-commit-config.yaml:

repos:
- repo: https://github.com/psf/black
  rev: 22.1.0
  hooks:
  - id: black

- repo: https://github.com/keewis/blackdoc
  rev: v0.3.4
  hooks:
  - id: blackdoc
    additional_dependencies: ["black==22.1.0"]
  - id: sync-black-version

where the sync-black-version hook will keep it in sync with the hook version of black. That way, I won't have to release a new version every time black does (although that might be better given that blackdoc is using some of the internal API of black).

max-sixty commented 2 years ago

I agree it shouldn't be in the setup.cfg requirements of blackdoc (that would pin the version of black that consumers use). What do you think about putting the additional_dependencies in the blackdoc pre-commit configs? Although it would mean the black version would lag a bit (until you did the next release), it would mean that people would get a decent option by default. Having a slightly old version of black for blackdoc is fine, but having inconsistent versions between people running it isn't good.

(TBC, it's completely your call and I'm asking because I use blackdoc in lots of repos and we hopefully know each other well enough for me to ask; it's fine to say "OK but no"! :) )

keewis commented 2 years ago

My main concern was that black would introduce changes to the code style, but I guess now that black is stable and the most important changes only land once per year this is not so important anymore. It would also avoid having the hook break because black (or tomli) changed, although that might also be avoided by increasing the test coverage...

keewis commented 2 years ago

notes to myself:

it looks like additional_dependencies (or dependencies of hooks in general) are a unsolved issue with pre-commit: they are only updated whenever a hook is installed or updated. I can probably write a github action (or a local hook that pre-commit.ci can run) to update black whenever that is necessary, and release shortly afterwards. That might require CalVer, though...