-
Notifications
You must be signed in to change notification settings - Fork 7k
[release test] edit shell script styling in run_release_test.sh #58670
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
Conversation
use double brackets as much as possible Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
| reason() { | ||
| # Keep in sync with e2e.py ExitCode enum | ||
| if [ "$1" -eq 0 ]; then | ||
| if [[ "$1" -eq 0 ]]; then |
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.
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.
Code Review
The pull request correctly updates the shell script to use [[ ... ]] instead of [ ... ], which is a good practice for bash scripts. However, I've identified two issues. A block of code for enabling debug mode appears to have been unintentionally removed. Additionally, the reason function has a bug that could lead to incorrect error reporting for unhandled exit codes. I've provided detailed comments and suggestions for both of these points.
…project#58670) use double brackets as much as possible Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
…project#58670) use double brackets as much as possible Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: YK <1811651+ykdojo@users.noreply.github.com>
…project#58670) use double brackets as much as possible Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
…project#58670) use double brackets as much as possible Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: Future-Outlier <eric901201@gmail.com>
use double brackets as much as possible