Refactor and shellcheck cleanup in report_errors.sh and check_style.sh #10797
Conversation
| ) | ||
| EXCLUDE_FILES_JOINED=$(printf "%s\|" "${EXCLUDE_FILES[@]}") | ||
| EXCLUDE_FILES_JOINED=${EXCLUDE_FILES_JOINED%??} | ||
|
|
||
| ( | ||
| REPO_ROOT="$(dirname "$0")"/.. | ||
| cd $REPO_ROOT | ||
| cd "$REPO_ROOT" || exit 1 |
There was a problem hiding this comment.
Switching to set -e would be better but I did not have enough time to test the script with it properly today so I decided to go with this for now.
| preparedGrep "#include \"" | egrep -v -e "license.h" -e "BuildInfo.h" # Use include with <> characters | ||
| preparedGrep "\<(if|for|while|switch)\(" # no space after "if", "for", "while" or "switch" | ||
| preparedGrep "\<for\>\s*\([^=]*\>\s:\s.*\)" # no space before range based for-loop | ||
| preparedGrep "\<if\>\s*\(.*\)\s*\{\s*$" # "{\n" on same line as "if" / "for" |
There was a problem hiding this comment.
The comment wasn't true so I removed the part about for. But why actually aren't we checking for? Is there any reason this isn't (if|for|while|switch)? I checked blame and it looks like it was if from the beginning.
There was a problem hiding this comment.
OK. But in that case I'll do it in a separate PR because this will require fixing the style issues that were passing though this check until now.
| echo $ERROR_MSG | ||
|
|
||
| if [ ! -z $CI ] | ||
| if [[ $CI == "true" ]] |
There was a problem hiding this comment.
Could also be $CI != "" but I see in CircleCI that this is set to true and I think this check is more readable.
cameel
force-pushed
the
refactor-and-shellcheck-cleanup-in-report-errors-sh
branch
from
54e3d8f to
f68cc14
Compare
- check_style.sh already does this
cameel
force-pushed
the
refactor-and-shellcheck-cleanup-in-report-errors-sh
branch
from
f68cc14 to
81f3c74
Compare
|
Oh, this one is actually not blocked by anything. Switching back to "ready to review". |
|
@leonardoalt We have an answer now. I'll change it but this PR is ready to merge now so I think it's better to merge it and update this rule in a new one. |
leonardoalt
deleted the
refactor-and-shellcheck-cleanup-in-report-errors-sh
branch
NOTE: This PR is based on #10792 and should not be merged before it.It's ondevelopnow.This is the extra, optional cleanup for the scripts for checking and reporting code style errors. It contains the relevant bits from #10585 and #10586 and a few more things. This is not everything that can be done here but that's the part I managed to finish today and is complete enough to be a PR of its own.