-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
refactor: start_docker() #1410
refactor: start_docker() #1410
Conversation
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.
Thanks for the pull request!
Can you also address similar ones in line number 268, 272 and 290?
[Line 268:](javascript:setPosition(268, 15))
if (( $EUID != 0 )); then
^-- [SC2004](https://www.shellcheck.net/wiki/SC2004) (style): $/${} is unnecessary on arithmetic variables.
[Line 272:](javascript:setPosition(272, 19))
if [[ $? -ne 0 ]] && ! $sudo_cmd -v; then
^-- [SC2181](https://www.shellcheck.net/wiki/SC2181) (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
[Line 290:](javascript:setPosition(290, 7))
if (( $EUID != 0 )); then
^-- [SC2004](https://www.shellcheck.net/wiki/SC2004) (style): $/${} is unnecessary on arithmetic variables.
[Line 307:](javascript:setPosition(307, 7))
if [[ $? -ne 0 ]]; then
^-- [SC2181](https://www.shellcheck.net/wiki/SC2181) (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
@prashant-shahi since there is no arithmatic calculation involved using |
Kudos, SonarCloud Quality Gate passed! |
@AkshayAwate |
Running a command and then checking its exit status with
$?
against 0 is redundant.More info can be found: https://www.shellcheck.net/wiki/SC2181
If this is good to go, then we can refactor other functions code too.
Thanks