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.

Sign up for GitHub

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

[CI] Fix shellcheck warning in pre-commit #50288

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+10 −11
Open

[CI] Fix shellcheck warning in pre-commit #50288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
261aeb1
fix shellcheck warning
owenowenisme Feb 6, 2025
b9a2c2c
add shellcheck to pre-commit hook
owenowenisme Feb 6, 2025
ab4960f
remove dup function in rllib_contrib_ci.sh
owenowenisme Feb 7, 2025
67f1eab
Merge branch 'master' into fix-shellcheck-warning
owenowenisme Feb 8, 2025
c27bfee
test using shellcheck-py
owenowenisme Feb 8, 2025
f652e09
Merge branch 'master' into fix-shellcheck-warning
owenowenisme Feb 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ repos:
- id: python-no-log-warn
- id: python-check-mock-methods

- repo: https://github.com/koalaman/shellcheck-precommit
rev: v0.9.0
- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.9.0.1
hooks:
- id: shellcheck
args: ['--exclude=1090,1091,2207']
Expand Down
1 change: 1 addition & 0 deletions ci/lint/lint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pre_commit() {
rst-inline-touching-normal
python-check-mock-methods
clang-format
shellcheck
docstyle
check-import-order
check-cpp-files-inclusion
Expand Down
8 changes: 6 additions & 2 deletions ci/ray_ci/macos/macos_ci_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export TORCHVISION_VERSION=0.15.2
build_x86_64() {
# Cleanup environments
rm -rf /tmp/bazel_event_logs
cleanup() { if [ "${BUILDKITE_PULL_REQUEST}" = "false" ]; then ./ci/build/upload_build_info.sh; fi }; trap cleanup EXIT
# shellcheck disable=SC2317
cleanup() { if [ "${BUILDKITE_PULL_REQUEST}" = "false" ]; then ./ci/build/upload_build_info.sh; fi }
trap cleanup EXIT
(which bazel && bazel clean) || true
# TODO(simon): make sure to change both PR and wheel builds
# Special setup for jar builds (will be installed to the machine instead)
Expand Down Expand Up @@ -54,7 +56,9 @@ build_x86_64() {
build_aarch64() {
# Cleanup environments
rm -rf /tmp/bazel_event_logs
cleanup() { if [ "${BUILDKITE_PULL_REQUEST}" = "false" ]; then ./ci/build/upload_build_info.sh; fi }; trap cleanup EXIT
# shellcheck disable=SC2317
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does here need SC2317?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cause here we are using trap to invoke cleanup function, and it will make the shellcheck think it is unreachable, but this is a false positive.

cleanup() { if [ "${BUILDKITE_PULL_REQUEST}" = "false" ]; then ./ci/build/upload_build_info.sh; fi }
trap cleanup EXIT
(which bazel && bazel clean) || true
brew install pkg-config nvm node || true
# TODO(simon): make sure to change both PR and wheel builds
Expand Down
6 changes: 0 additions & 6 deletions ci/ray_ci/rllib_contrib/rllib_contrib_ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,6 @@ test_slate_q() {
bazel test --config=ci $(./ci/run/bazel_export_options) --build_tests_only --test_tag_filters=-flaky,learning_tests --test_arg=--framework=torch rllib_contrib/slate_q/...
}

test_td3() {
build "td3"
# BAZEL (learning and compilation) tests:
bazel test --config=ci $(./ci/run/bazel_export_options) --build_tests_only --test_tag_filters=-flaky rllib_contrib/td3/...
}

test_a3c() {
build "a3c"
# BAZEL (learning and compilation) tests:
Expand Down
2 changes: 1 addition & 1 deletion setup_hooks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash
set -eu

# This stops git rev-parse from failing if we run this from the .git directory
Expand Down