Repository navigation
fix(comments): keep a writer failure apart from the git grep status - #16
Merged
Merged
Conversation
The check treats exit 1 from git grep as "no match". A redirection that fails after the work file was truncated also exits 1, so a later write failure could leave an empty result and skip the scan without an error. Write each command's own status to a separate file from inside the redirected block. If that file is missing or not a single integer, the redirection or the status write failed, and the check exits 1 with an error. Otherwise the existing 0, 1 and greater-than-1 handling applies. The docstring scan step already treats any non-zero status as an error, so it needs no change. The self-test gains a case for each wrapped call. A PATH shim makes the status file unwritable just as the targeted git grep starts, with no seam in the script. Refs dmfdeploy/dmfdeploy#768 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The review-history check reads exit status 1 from
git grepas "no match". A failed output redirection also exits 1, so if writing a work file failed after the check had truncated it, the result came back empty and the scan was skipped without an error.git grepcall writes its own exit status to a separate file, from inside the redirected block.The scanner matches the other repos' copies. Only this repo's
excludelist differs.Testing
bin/check-review-comments.sh --self-testpasses. It has a case for each wrapped call, where agitwrapper onPATHmakes the status file unwritable as the targetedgit grepstarts. Both cases fail with exit 0 when a missing status is read as "no match".bin/check-review-comments.shgives the same result as before this change.shellcheck bin/check-review-comments.shis clean.Refs dmfdeploy/dmfdeploy#768