-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Meta: Add trailing-whitespace and end-of-file-fixer to pre-commit config #4067
Comments
Fine by me. It will affect 243 files, better to do them in one go to avoid over-spamming. Are there any other similar rules we should add at the same time? 243 files
|
I tried opening a PR this morning and 54 reviewers were added because of |
Can we exclude those files from the hook? Then only new PEPs will be affected. |
Not sure how to do that without having to enumerate all of the files currently in the repo. If anyone has any ideas for this, I'd be happy to do it. |
For a one-off with a quick merge, I think it's okay. We get worse on mistake PRs quite regularly!
The Exclude something like |
Right! This takes it down to 19 requested reviews. Should we go ahead and do that? |
Oops sorry, missed this - I opened #4078 as an illustration of what I meant but it is pretty much just replicating Hugo's point. An alternative would be to enumerate every number, which wouldn't be too painful. |
A workaround we sometimes use is to keep the PR in draft until it's ready, then quickly take it out of draft and merge it. Everyone will still get a notification, but they'll see the PR is already merged and they won't have to review it. |
I was trying to remove all trailing whitespace from PEP 750 and noticed that we currently do not have
trailing-whitespace
andend-of-file-fixer
in our pre-commit config. I think it makes sense to include these. From thepre-commit-hooks
README:Do people have objections?
The text was updated successfully, but these errors were encountered: