You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "xumingming (via GitHub)" <gi...@apache.org> on 2024/08/18 03:16:25 UTC
[PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
xumingming opened a new pull request, #6907:
URL: https://github.com/apache/incubator-gluten/pull/6907
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
(Fixes: \#6900)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #6907:
URL: https://github.com/apache/incubator-gluten/pull/6907#issuecomment-2299368836
> @FelixYBW The reason is as follows:
>
> 1. When I change some c++ code, and submit the PR, the PR will fail at code style check.
> 2. I noticed there is `dev/formatcppcode.sh`, but it only support Linux
> 3. After make `dev/formatcppcode.sh` support macOS, I found that clang-format on macOS(which is of version 18) behaves differently from clang-format-15 on Linux.
> 4. And there seems no easy way to install clang-format-15 on macOS.
>
> Do you have suggestions?
Different OS has different default clang-format version. e.g. I'm using Ubuntu20.04 which has clang-format-12. Do 12 and 18 have any difference on format? format-12 works well on my side so far.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "xumingming (via GitHub)" <gi...@apache.org>.
xumingming closed pull request #6907: [GLUTEN-6900] Use clang-format-18
URL: https://github.com/apache/incubator-gluten/pull/6907
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "xumingming (via GitHub)" <gi...@apache.org>.
xumingming commented on PR #6907:
URL: https://github.com/apache/incubator-gluten/pull/6907#issuecomment-2298776052
@FelixYBW The reason is as follows:
1. When I change some c++ code, and submit the PR, the PR will fail at code style check.
2. I noticed there is `dev/formatcppcode.sh`, but it only support Linux
3. After make `dev/formatcppcode.sh` support macOS, I found that clang-format on macOS(which is of version 18) behaves differently from clang-format-15 on Linux.
Do you have suggestions?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on code in PR #6907:
URL: https://github.com/apache/incubator-gluten/pull/6907#discussion_r1723681654
##########
dev/formatcppcode.sh:
##########
@@ -1,13 +1,25 @@
cd `dirname $0`
-# Check if clang-format-15 is installed
-if ! command -v clang-format-15 &> /dev/null
-then
- echo "clang-format-15 could not be found"
- echo "Installing clang-format-15..."
- sudo apt update
- sudo apt install clang-format-15
+CLANG_FORMAT_CMD=""
+if [ "$(uname)" == "Darwin" ]; then
+ CLANG_FORMAT_CMD="clang-format"
+ if ! command -v $CLANG_FORMAT_CMD &> /dev/null
+ then
+ echo "$CLANG_FORMAT_CMD could not be found"
+ echo "Installing $CLANG_FORMAT_CMD..."
+ sudo brew install $CLANG_FORMAT_CMD
+ fi
+else
+ CLANG_FORMAT_CMD="clang-format-18"
+ if ! command -v $CLANG_FORMAT_CMD &> /dev/null
+ then
+ echo "$CLANG_FORMAT_CMD could not be found"
+ echo "Installing $CLANG_FORMAT_CMD..."
+ sudo apt update
+ sudo apt install $CLANG_FORMAT_CMD
+ fi
fi
Review Comment:
Let's remove the install commands here. It's only for darwin and ubuntu.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "xumingming (via GitHub)" <gi...@apache.org>.
xumingming commented on PR #6907:
URL: https://github.com/apache/incubator-gluten/pull/6907#issuecomment-2308374304
> > @FelixYBW The reason is as follows:
> >
> > 1. When I change some c++ code, and submit the PR, the PR will fail at code style check.
> > 2. I noticed there is `dev/formatcppcode.sh`, but it only support Linux
> > 3. After make `dev/formatcppcode.sh` support macOS, I found that clang-format on macOS(which is of version 18) behaves differently from clang-format-15 on Linux.
> > 4. And there seems no easy way to install clang-format-15 on macOS.
> >
> > Do you have suggestions?
>
> Different OS has different default clang-format version. e.g. I'm using Ubuntu20.04 which has clang-format-12. Do 12 and 18 have any difference on format? format-12 works well on my side so far.
Yes, they have many difference. Seems not easy to get a same output on macOS, closing it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #6907:
URL: https://github.com/apache/incubator-gluten/pull/6907#issuecomment-2295161122
Why do we need to upgrade 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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-6900] Use clang-format-18 [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW closed pull request #6907: [GLUTEN-6900] Use clang-format-18
URL: https://github.com/apache/incubator-gluten/pull/6907
--
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.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org