-
Notifications
You must be signed in to change notification settings - Fork 33
Removemigrateshellck #755
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
Removemigrateshellck #755
Conversation
|
|
|
|
|
|
|
|
…d-only file system
|
|
|
|
|
|
if [ -n "$label" ]; then | ||
log_debug "Applying label [$label] to newly created secret [$secret_name]" | ||
# shellcheck disable=SC2086 | ||
# quoting $label results in invalid kubectl syntax |
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 wonder if we should have require an explanation when disabling any shellcheck directive in the code? Adding an explanation after the disable directive (e.g. #shellcheck disable=SC2086 quoting $label results in invalid kubectl syntax
) is NOT valid, so I had to add a 2nd comment. The Shellcheck doc shows including both comments on the same line as acceptable alternative, i.e. #shellcheck disable=SC2086 #quoting $label results in invalid kubectl syntax
. That might be cleaner/more readable. Thoughts?
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 think that's a great idea. I think I actually prefer what you did here, it is easier to read if it is on a sep line
No description provided.