-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CI: Format isort output for azure #29654
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
Changes from 2 commits
1ceae3c
04c533d
f1a623b
6aadcf1
9c25358
2bd6f23
da5dea0
b0ad4b5
28be684
984a653
232e815
54b141c
64498ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,7 +109,13 @@ if [[ -z "$CHECK" || "$CHECK" == "lint" ]]; then | |
|
||
# Imports - Check formatting using isort see setup.cfg for settings | ||
MSG='Check import format using isort ' ; echo $MSG | ||
isort --recursive --check-only pandas asv_bench | ||
set -o pipefail | ||
isort_cmd="isort --recursive --check-only pandas asv_bench" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the convention is to have environment variables in bash in upper case. |
||
if [[ "$AZURE" == "true" ]]; then | ||
eval $isort_cmd | awk '{ORS=""} {print "##vso[task.logissue type=error;sourcepath="$2";]"} {$1=$2=""; print $0}' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an opinion, but I'd personally prefer something like: if [[ "$AZURE" == "true" ]]; then
FORMAT_ISORT="awk '{ORS=""} {print "##vso[task.logissue type=error;sourcepath="$2";]"} {$1=$2=""; print $0}'"
else
FORMAT_ISORT="cat"
fi
isort --recursive --check-only pandas asv_bench | eval $FORMAT_ISORT Not a huge difference, but I think it makes more clear what's going on, since the line that actually does things is just one, shorter, and more explicit on what it does |
||
else | ||
eval $isort_cmd | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This indentation doesn't much the previous eval. |
||
fi | ||
RET=$(($RET + $?)) ; echo $MSG "DONE" | ||
|
||
fi | ||
|
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 you should disable when it's not needed anymore. Otherwise things in other part of the script can fail, and this is not expected to be active by default.