interrogate icon indicating copy to clipboard operation
interrogate copied to clipboard

feat(pre-commit-hooks): add setuptools to additional dependencies

Open ericmjl opened this issue 2 years ago • 2 comments

Hey, I just made a Pull Request!

Description

This PR adds setuptools to the additional dependencies in the pre-commit-hooks.yaml file. This change ensures that setuptools is available during the execution of pre-commit hooks, which can be crucial for certain Python hooks that rely on setuptools.

Motivation and Context

Resolves #162

Have you tested this? If so, how?

I have not yet tested this change, but based on reading the documentation, I have moderately high confidence that this is the right fix for #162.

Checklist for PR author(s)

  • [x] Changes are covered by unit tests (no major decrease in code coverage %).
  • [x] All tests pass.
  • [x] Docstring coverage is 100% via tox -e docs or interrogate -c pyproject.toml (I mean, we should set a good example :smile:).
  • [x] Updates to documentation:
    • [x] Document any relevant additions/changes in README.rst.
    • [x] Manually update both the README.rst and docs/index.rst for any new/changed CLI flags.
    • [x] Any changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives. Find the appropriate next version in the project's __init__.py file.

Release note

Add `setuptools` to the additional dependencies in the pre-commit-hooks.yaml file.
This change ensures that setuptools is available during the execution of pre-commit hooks,
which is explictly needed when running with Python 3.12.

ericmjl avatar Oct 29 '23 15:10 ericmjl

(I'm not involved with this project, just a curious person who followed links from your blog post)

I came here to suggest that switching from pkg_resources to importlib.resources would be a more "proper" fix in the long term - a lot of people seem not to know about importlib.resources as a replacement for pkg_resources. But it looks like that's already been done in commit 3aa94a6a9d6dca0a015d97ef389feb788cb466dd. So I suspect this change may not be needed anymore.

diazona avatar Nov 01 '23 16:11 diazona

Ah! That is something I didn’t know! Thank you very much, @diazona, I definitely learned something new today.

ericmjl avatar Nov 01 '23 21:11 ericmjl

@econchick i think we can close out this PR, no need to merge it. What do you think?

ericmjl avatar Apr 06 '24 23:04 ericmjl

@ericmjl agreed - ty!

econchick avatar Apr 08 '24 17:04 econchick