You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2024/03/14 09:08:41 UTC
[PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
kou opened a new pull request, #40545:
URL: https://github.com/apache/arrow/pull/40545
### Rationale for this change
We can maintain cmake-format related configuration by just listing target files and excluded files in YAML instead of writing not only listing paths but also extra codes in Python by using pre-commit.
### What changes are included in this PR?
Add cmake-format related pre-commit configuration and remove cmake-format related code from Archery.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Add cmake-format configuration to pre-commit [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #40545:
URL: https://github.com/apache/arrow/pull/40545
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-2000557195
> If you don't like removing `cmake_format` related codes from Archery before `pre-commit run -a` can replace `archery lint` entirely, I'll remove Archery related changes from this PR.
Please do. It would be annoying to have to use two different commands for linting.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-1998490538
> does this mean I have to install and run an additional tool to properly format CMake changes?
Yes. You need to install `pre-commit` by `conda install -c conda-forge pre-commit`/`pip install pre-commit`/`sudo apt install -y pre-commit` (Ubuntu 22.04+) and run `pre-commit run -a` instead of `archery lint`. Note that you don't need to install `cmake_format` separately like `pip install -e 'dev/archery[lint]'`.
> The selling point of `archery lint` is that it handles everything by itself.
This means that `archery lint` runs all lints (`cmake_format`, `clang-format` and so on), right?
`pre-commit run -a` also does it eventually. I'll open PRs for it. For example, I'll add configurations of `clang-format` for `cpp/` and `python/` to `.pre-commit-config.yaml` like existing configuration of `c_glib/`: https://github.com/apache/arrow/blob/eecd7797f1d789551fbb000a93504182c47b6baa/.pre-commit-config.yaml#L55-L61
If you don't like removing `cmake_format` related codes from Archery before `pre-commit run -a` can replace `archery lint` entirely, I'll remove Archery related changes from this PR.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-1997076311
Hmm, does this mean I have to install and run an additional tool to properly format CMake changes? The selling point of `archery lint` is that it handles everything by itself.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-2001885209
+1
It's executed:
https://github.com/apache/arrow/actions/runs/8306057631/job/22733676937?pr=40545#step:6:17
```text
CMake Format.............................................................Passed
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-2001880068
OK. I've reverted Archery changes.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Add cmake-format configuration to pre-commit [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-2002406520
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 555ce5c667c67a7fa033c3903685c0de888a182d.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/22752605971) has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40544: [Dev] Use pre-commit for cmake-format [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40545:
URL: https://github.com/apache/arrow/pull/40545#issuecomment-1996960105
:warning: GitHub issue #40544 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org