-
-
Notifications
You must be signed in to change notification settings - Fork 631
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
Apply final set of Shellcheck fixes and turn on in CI #7832
Apply final set of Shellcheck fixes and turn on in CI #7832
Conversation
They are too difficult to get right. Better to just skip them.
Can't figure out how to fix.
@@ -6,4 +6,4 @@ | |||
# Check for copies (-C) and moves (-M), so we don't get false positives when people do | |||
# refactorings. -l50 bounds the time git takes to search for these non-additions. | |||
# See git-diff(1) and https://stackoverflow.com/a/2299672/2518889 for discussion of these options. | |||
exec git diff --cached --name-only --diff-filter=A -C -M -l50 | |||
exec git --no-pager diff --cached --name-only --diff-filter=A -C -M -l50 |
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.
Depending on how the user has their git set up, this script would sometimes fail. For example, my git for some reason defaults to opening git diff
through less
.
Now, the script will work no matter what.
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 didn't know about this argument! Note that many programs (including git
) will respect the PAGER
env var, and I've usually done this by running the command with PAGER=cat
-- but this is definitely better!
@@ -6,4 +6,4 @@ | |||
# Check for copies (-C) and moves (-M), so we don't get false positives when people do | |||
# refactorings. -l50 bounds the time git takes to search for these non-additions. | |||
# See git-diff(1) and https://stackoverflow.com/a/2299672/2518889 for discussion of these options. | |||
exec git diff --cached --name-only --diff-filter=A -C -M -l50 | |||
exec git --no-pager diff --cached --name-only --diff-filter=A -C -M -l50 |
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 didn't know about this argument! Note that many programs (including git
) will respect the PAGER
env var, and I've usually done this by running the command with PAGER=cat
-- but this is definitely better!
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'm learning a whole lot with these changes :) Thanks!
Builds off of the work from #7698 and #7719.
Several of the problems are ignored because they are very difficult to correctly fix, and the behavior is not broken when ran locally and in CI so the cost of fixing is not deemed worth it.