Give line numbers in git-grep-based lints #53733
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💊 CI failures summary and remediationsAs of commit d7f134c (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
There was a problem hiding this comment.
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
malfet
approved these changes
Mar 10, 2021
xsacha
pushed a commit
to xsacha/pytorch
that referenced
this pull request
Mar 31, 2021
Summary: Meant to make tasks like pytorch#53728 easier. The `-n` flag enables line numbers, and the `-o` flag reduces noise by only showing the part of the line that matched (which in this case is just the trailing whitespace). Pull Request resolved: pytorch#53733 Test Plan: ``` $ git checkout e937db5 ``` Before: ``` $ (! git grep -I -l ' $' -- . ':(exclude)**/contrib/**' ':(exclude)third_party' || (echo "The above files have trailing spaces; please remove them"; false)) aten/src/ATen/native/cuda/BatchLinearAlgebra.cu The above files have trailing spaces; please remove them ``` After: ``` $ (! git grep -I -no ' $' -- . ':(exclude)**/contrib/**' ':(exclude)third_party' || (echo "The above files have trailing spaces; please remove them"; false)) aten/src/ATen/native/cuda/BatchLinearAlgebra.cu:1972: The above files have trailing spaces; please remove them ``` Reviewed By: mruberry Differential Revision: D26953538 Pulled By: samestep fbshipit-source-id: 5f7d48b79f1a02e5e5a09fe00316ec350cfc340e
facebook-github-bot
pushed a commit
that referenced
this pull request
Apr 8, 2021
Summary: malfet found a couple of these in #55346; this PR removes the rest and adds a lint that prevents them from being accidentally added again in the future. It also removes the `-o` flag added in #53733 (which was unnecessarily hiding context without reducing the number of lines of output), and updates the lint error messages to reflect that the individual line numbers are shown in the logs. Pull Request resolved: #55465 Test Plan: The "Lint / quick-checks" job in GitHub Actions should succeed on this PR. To verify that the lint does correctly find and error on non-breaking spaces, checkout ece0751 and run it locally: ```sh (! git --no-pager grep -In $'\u00a0' -- . || (echo "The above lines have non-breaking spaces (U+00A0); please convert them to spaces (U+0020)"; false)) ``` It should print over a hundred lines of output and exit with status 1. Reviewed By: janeyx99 Differential Revision: D27622136 Pulled By: samestep fbshipit-source-id: e7ffd5a9519093e7a0ffdf55e9291f63e21ce841
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Meant to make tasks like #53728 easier. The
-nflag enables line numbers, and the-oflag reduces noise by only showing the part of the line that matched (which in this case is just the trailing whitespace).Test plan:
Before:
After: