Skip to content

Document pre-commit with --force-exclude#3109

Closed
Ttibsi wants to merge 1 commit into
psf:mainfrom
Ttibsi:document-force-exclude
Closed

Document pre-commit with --force-exclude#3109
Ttibsi wants to merge 1 commit into
psf:mainfrom
Ttibsi:document-force-exclude

Conversation

@Ttibsi
Copy link
Copy Markdown
Contributor

@Ttibsi Ttibsi commented Jun 6, 2022

Description

Closes #3015

Checklist - did you ...

  • [/] Add a CHANGELOG entry if necessary? (I don't believe this is required - please let me know if it is and I can amend my PR with an update)
  • [/] Add / update tests if necessary?
  • [/] Add new / update outdated documentation?

@ichard26 ichard26 added T: documentation Improvements to the docs (e.g. new topic, correction, etc) ci: skip news Pull requests that don't need a changelog entry. labels Jun 6, 2022
Copy link
Copy Markdown
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and works for me.

Only enhancement I could think of is maybe showing or at least stating how to specify multiple directories - i.e. is it two --force-exclude or is it comma separated paths etc.

Comment on lines +41 to +42
args:
- --force-exclude=tests/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the best place to put this? if you're using pre-commit, you might as well use pre-commit's own exclude

I think the more common use-case for force-exclude is for putting it in pyproject.toml, so that this config stay synced between when running black via pre-commit or via the command-line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured it would be better to document Black's own way of doing this - we can easily add another line talking about pre-commit's exclude. I don't have much experience with pyproject.toml, so anything I add to that will need a second pair of eyes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @MarcoGorelli, using --force-exclude in pre-commit-config.yaml is a bit idiosyncratic. Either you use it in pyproject.toml or you use pre-commit's own exclude configuration. I'd personally recommend pre-commit's exclude first, and then --force-exclude, as the latter can be overkill.

@ichard26 ichard26 self-requested a review June 6, 2022 17:14
@ichard26 ichard26 self-assigned this Jul 7, 2022
Copy link
Copy Markdown
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Reading others comments, I agree people should just use pyproject.toml black config to exclude directories rather than the CLI. Here is an example of how we even do it with black on the black repo: https://github.com/psf/black/blob/main/pyproject.toml#L12

I think due to this should we close this PR as we already have this documented in black docs. If you feel it could be better I'd take a PR improving that.

@mscheltienne
Copy link
Copy Markdown

Thanks for this. Reading others comments, I agree people should just use pyproject.toml black config to exclude directories rather than the CLI. Here is an example of how we even do it with black on the black repo: https://github.com/psf/black/blob/main/pyproject.toml#L12

Aren't the entries in exclude and in extend-exclude ignored by pre-commit? And then you now also need to define force-exclude=exclude + extend-exclude in your pyproject.toml?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: skip news Pull requests that don't need a changelog entry. T: documentation Improvements to the docs (e.g. new topic, correction, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document using pre-commit and how --force-exclude can help

5 participants