You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/09/14 18:15:05 UTC
[GitHub] [incubator-tvm] comaniac opened a new pull request #6469: [Formatting] Fix black script for Python formatting
comaniac opened a new pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469
This PR fixes some issues I encountered when running `git-black.sh` on `bash`:
1. A syntax error (an unexpected ")") in a message. This seems fine on `zsh` but causes crash on `bash`.
2. A guard to prevent failure when no changes in Python file (`$FILES` would be an unbound variable).
3. Remove `HEAD` in the command of listing changed Python files so that it will also cover upstaged changes.
While the first two points may fail the CI, the third point is optional. I just feel that it's more convenient for people to format their changes before making a commit. I could revert this one if you folks don't prefer to have it.
cc @jroesch @tqchen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen merged pull request #6469: [Formatting] Fix black script for Python formatting
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6469: [Formatting] Fix black script for Python formatting
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469#issuecomment-692385335
Thanks @comaniac @junrushao1994 @jroesch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6469: [Formatting] Fix black script for Python formatting
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469#discussion_r488222136
##########
File path: tests/lint/git-black.sh
##########
@@ -46,16 +46,20 @@ fi
echo "Version Information: $(black --version)"
# Compute Python files which changed to compare.
-IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only HEAD $1 -- "*.py" "*.pyi") || true
-echo "read returned $?"
+IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only $1 -- "*.py" "*.pyi") || true
+echo "Read returned $?"
+if [ -z ${FILES+x} ]; then
+ echo "No changes in Python files"
+ exit 0
+fi
echo "Files: $FILES"
if [[ ${INPLACE_FORMAT} -eq 1 ]]; then
- echo "Running black on Python files against revision" $1):
+ echo "Running black on Python files against revision" $1:
CMD=( "black" "${FILES[@]}" )
echo "${CMD[@]}"
"${CMD[@]}"
else
echo "Running black in checking mode"
- black --diff --check
+ black --diff --check ${FILES[@]}
Review comment:
I think incremental is fine in most cases, since it is also consistent with git-clang-format
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] jroesch commented on a change in pull request #6469: [Formatting] Fix black script for Python formatting
Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469#discussion_r488171309
##########
File path: tests/lint/git-black.sh
##########
@@ -46,16 +46,20 @@ fi
echo "Version Information: $(black --version)"
# Compute Python files which changed to compare.
-IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only HEAD $1 -- "*.py" "*.pyi") || true
-echo "read returned $?"
+IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only $1 -- "*.py" "*.pyi") || true
+echo "Read returned $?"
+if [ -z ${FILES+x} ]; then
+ echo "No changes in Python files"
+ exit 0
+fi
echo "Files: $FILES"
if [[ ${INPLACE_FORMAT} -eq 1 ]]; then
- echo "Running black on Python files against revision" $1):
+ echo "Running black on Python files against revision" $1:
CMD=( "black" "${FILES[@]}" )
echo "${CMD[@]}"
"${CMD[@]}"
else
echo "Running black in checking mode"
- black --diff --check
+ black --diff --check ${FILES[@]}
Review comment:
I was intentionally having this check the entire repo, not sure if we want to be incremental when doing the full check vs. being incremental when we are formatting. cc @tqchen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6469: [Formatting] Fix black script for Python formatting
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469#discussion_r488180925
##########
File path: tests/lint/git-black.sh
##########
@@ -46,16 +46,20 @@ fi
echo "Version Information: $(black --version)"
# Compute Python files which changed to compare.
-IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only HEAD $1 -- "*.py" "*.pyi") || true
-echo "read returned $?"
+IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only $1 -- "*.py" "*.pyi") || true
+echo "Read returned $?"
+if [ -z ${FILES+x} ]; then
+ echo "No changes in Python files"
+ exit 0
+fi
echo "Files: $FILES"
if [[ ${INPLACE_FORMAT} -eq 1 ]]; then
- echo "Running black on Python files against revision" $1):
+ echo "Running black on Python files against revision" $1:
CMD=( "black" "${FILES[@]}" )
echo "${CMD[@]}"
"${CMD[@]}"
else
echo "Running black in checking mode"
- black --diff --check
+ black --diff --check ${FILES[@]}
Review comment:
It looks like I have to provide something; otherwise it will just pop the following message in all cases.
```
ubuntu@~/tvm> black --diff --check
No Path provided. Nothing to do 😴
```
Black version 20.8b1. Not sure if this is due to my local environment issue or something else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6469: [Formatting] Fix black script for Python formatting
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6469:
URL: https://github.com/apache/incubator-tvm/pull/6469#discussion_r488222136
##########
File path: tests/lint/git-black.sh
##########
@@ -46,16 +46,20 @@ fi
echo "Version Information: $(black --version)"
# Compute Python files which changed to compare.
-IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only HEAD $1 -- "*.py" "*.pyi") || true
-echo "read returned $?"
+IFS=$'\n' read -a FILES -d'\n' < <(git diff --name-only $1 -- "*.py" "*.pyi") || true
+echo "Read returned $?"
+if [ -z ${FILES+x} ]; then
+ echo "No changes in Python files"
+ exit 0
+fi
echo "Files: $FILES"
if [[ ${INPLACE_FORMAT} -eq 1 ]]; then
- echo "Running black on Python files against revision" $1):
+ echo "Running black on Python files against revision" $1:
CMD=( "black" "${FILES[@]}" )
echo "${CMD[@]}"
"${CMD[@]}"
else
echo "Running black in checking mode"
- black --diff --check
+ black --diff --check ${FILES[@]}
Review comment:
I think incremental is fine in most cases
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org