vet: add check to ensure terminating newline #7645
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7645 +/- ##
==========================================
- Coverage 81.85% 81.75% -0.11%
==========================================
Files 361 361
Lines 27822 27822
==========================================
- Hits 22773 22745 -28
- Misses 3851 3872 +21
- Partials 1198 1205 +7 |
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:]]$' | |||
|
|
|||
| # - Ensure that all files have a terminating newline. | |||
| git grep -l '$' -- "*.txt" "*.md" | xargs -I {} sh -c '[ -n "$(tail -c 1 "{}")" ] && echo "{}: No terminating new line found"' | |||
There was a problem hiding this comment.
you have to negate the output. git grep returns exit status 1 when success so we have to negate to get 0. do not git grep....
There was a problem hiding this comment.
We should apply this restriction to all files in our repo I think, not just txt/md files. (We don't check in any binary files, at least not currently, so no need to worry about that.) So the first part can be git ls-files instead of a grep.
There was a problem hiding this comment.
Also to prevent your sh command from erroring, you can do && echo "{}: ..." || true (but maybe there's a better way?).
There was a problem hiding this comment.
We should apply this restriction to all files in our repo I think, not just txt/md files. (We don't check in any binary files, at least not currently, so no need to worry about that.) So the first part can be
git ls-filesinstead of agrep.
I noticed that the g3 presubmits only mandate terminating newline in text files, so I thought of following that, but I will change it to check in all the files
There was a problem hiding this comment.
Also to prevent your
shcommand from erroring, you can do&& echo "{}: ..." || true(but maybe there's a better way?).
Added specific checks forsh command errors by avoiding running on empty files and checking permissions and readability
|
Adding newline to the following files :
|
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:]]$' | |||
|
|
|||
| # - Ensure that all files have a terminating newline. | |||
| ! git ls-files | xargs -r -I {} sh -c '[ -n "$(tail -c 1 "{}" 2>/dev/null)" ] && echo "{}: No terminating new line found"' | |||
There was a problem hiding this comment.
nit: since everywhere we are using not, let's continue to use that instead of !
There was a problem hiding this comment.
suggestion for echo: missing a terminating newline
|
Please fix the merge conflict. Thanks. |
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:]]$' | |||
|
|
|||
| # - Ensure that all files have a terminating newline. | |||
| git ls-files | not xargs -r -I {} sh -c '[ -n "$(tail -c 1 "{}" 2>/dev/null)" ] && echo "{}: No terminating new line found"' | fail_on_output | |||
There was a problem hiding this comment.
I don't think you want this -r since there should always be results from git ls-files.
RELEASE NOTES: None