-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
vet : add check for tabs in text files #7678
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7678 +/- ##
==========================================
- Coverage 82.01% 81.86% -0.16%
==========================================
Files 361 361
Lines 27829 27822 -7
==========================================
- Hits 22825 22777 -48
- Misses 3821 3850 +29
- Partials 1183 1195 +12 |
scripts/vet.sh
Outdated
@@ -70,6 +70,9 @@ not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- | |||
# - Ensure that no trailing spaces are found. | |||
not git grep '[[:blank:]]$' | |||
|
|||
# - Check for tabs in Markdown files | |||
git grep $'\t' -- '*.md' | fail_on_output || echo "No tabs found in Markdown files." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we be echoing only for failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might be enough not git grep $'\t' -- '*.md'
because command and comment is self explanatory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
scripts/vet.sh
Outdated
@@ -70,6 +70,9 @@ not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- | |||
# - Ensure that no trailing spaces are found. | |||
not git grep '[[:blank:]]$' | |||
|
|||
# - Check for tabs in Markdown files | |||
not git grep $'\t' -- '*.md' | fail_on_output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why pipe to fail_on_output
? Inverting the result of grep
should be sufficient? I.e. grep
exits with 0 if there is a match and 1 if there is not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -70,6 +70,9 @@ not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- | |||
# - Ensure that no trailing spaces are found. | |||
not git grep '[[:blank:]]$' | |||
|
|||
# - Ensure that no tabs are found in markdown files. | |||
not git grep $'\t' -- '*.md' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/Should we do this for all files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean prohibit tab code files? I think the IDE might be doing that already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could be using an editor that doesn't do that. But looks like gofmt
uses tabs for indentation. See: https://pkg.go.dev/cmd/gofmt. And we run gofmt
in our vet script. So, we should be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go pretty famously uses tabs instead of spaces. Limiting to *.md SGTM.
RELEASE NOTES: None