Skip to content
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

Merged
merged 16 commits into from Jun 3, 2019

Conversation

Projects
None yet
4 participants
@Eric-Arellano
Copy link
Contributor

commented Jun 1, 2019

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.

@@ -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

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Jun 1, 2019

Author Contributor

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.

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Jun 3, 2019

Contributor

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!

Show resolved Hide resolved build-support/bin/native/bootstrap_rust.sh Outdated
Show resolved Hide resolved pants
Show resolved Hide resolved src/docs/publish_via_git.sh
Show resolved Hide resolved build-support/bin/check_rust_formatting.sh
@@ -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

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Jun 3, 2019

Contributor

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!

Show resolved Hide resolved build-support/bin/native/bootstrap_rust.sh Outdated
Show resolved Hide resolved build-support/bin/check_rust_formatting.sh
Show resolved Hide resolved build-support/bin/release.sh Outdated
Show resolved Hide resolved src/docs/publish_via_git.sh
Show resolved Hide resolved build-support/bin/native/bootstrap_rust.sh Outdated
Show resolved Hide resolved src/docs/publish_via_git.sh
@blorente
Copy link
Contributor

left a comment

I'm learning a whole lot with these changes :) Thanks!

Show resolved Hide resolved build-support/bin/check_rust_formatting.sh
Show resolved Hide resolved build-support/bin/native/bootstrap_rust.sh Outdated
Show resolved Hide resolved pants

@Eric-Arellano Eric-Arellano merged commit e2b18b9 into pantsbuild:master Jun 3, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:shellcheck-round-3 branch Jun 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.