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 2025/06/03 05:15:59 UTC
[PR] GH-46528: [CI][Dev] Remove "archery lint" [arrow]
kou opened a new pull request, #46686:
URL: https://github.com/apache/arrow/pull/46686
### Rationale for this change
We can use pre-commit instead of "archery lint" for all linters/formatters.
### What changes are included in this PR?
* Remove "archery lint" CI job
* Remove "archery lint" related codes including IWYU and clang-tidy because IWYU and clang-tidy aren't used for now
* Remove "archery lint" related docs including IWYU and clang-tidy related docs
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
It's only affected to developers.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2129176670
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
We should leave in information about how to run it other than with pre-commit. Many R users are more comfortable with running `lintr` directly, so it'd be great to give them both options.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2144108216
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
https://github.com/apache/arrow/pull/46799 will fix this.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2129173815
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
Review Comment:
We shouldn't remove the above line, it's useful information
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2129173815
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
Review Comment:
We shouldn't remove the above line, it's useful information
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2129176670
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
We should leave in information about how to run it other than with pre-commit. Many R users are more comfortable with running `lintr` directly as it's much simpler (I've personally had issues getting pre-commit set up so won't be the only one), so it'd be great to give them both options.
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
-
-```r
-lintr::lint_package("arrow/r")
+```bash
+pre-commit run --show-diff-on-failure --color=always --all-files r
```
-You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
Review Comment:
Let's leave this in too
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2144352374
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
OK. I'll merge this.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2122779895
##########
ci/scripts/install_iwyu.sh:
##########
Review Comment:
We'll revisit IWYU in the future.
It's tracked by https://github.com/apache/arrow/issues/46543 .
##########
docs/source/developers/guide/step_by_step/styling.rst:
##########
@@ -31,14 +31,7 @@ linters and styling of the code.
.. tab-item:: PyArrow
- We use flake8 linting for styling issues in Python. To help
- developers check styling of the code, among other common
- development tasks, :ref:`Archery utility<archery>` tool was
- developed within Apache Arrow.
-
- The instructions on how to set up and use Archery
- can be found in the Coding Style section of the
- :ref:`python-development`.
+ :ref:`python-coding-style`.
Review Comment:
This ID is added in this PR.
##########
cpp/build-support/run_clang_tidy.py:
##########
Review Comment:
We'll revisit clang-tidy in the future.
It's tracked by https://github.com/apache/arrow/issues/46542 .
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
-
-```r
-lintr::lint_package("arrow/r")
+```bash
+pre-commit run --show-diff-on-failure --color=always --all-files r
```
-You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
-
-Run the styler locally either via Makefile commands:
+See also the following subsections our styling and lint details for R and C++ codes.
-```bash
-make style # (for only the files changed)
-make style-all # (for all files)
-```
+### R code
-or in R:
+The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
-```r
-# note the file that should not be styled
-styler::style_pkg(exclude_files = c("data-raw/codegen.R"))
-```
+You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
Review Comment:
We'll use Air in the future.
It's traced by https://github.com/apache/arrow/issues/46592 .
##########
docs/source/developers/cpp/development.rst:
##########
@@ -102,128 +102,13 @@ following checks:
compiler warnings with ``-DBUILD_WARNING_LEVEL=CHECKIN``. Note that
there are classes of warnings (such as ``-Wdocumentation``, see more
on this below) that are not caught by ``gcc``.
-* Passes various C++ (and others) style checks, checked with the ``lint``
- subcommand to :ref:`Archery <archery>`. This can also be fixed locally
- by running ``archery lint --cpplint --clang-format --clang-tidy --fix``.
-* CMake files pass style checks, can be fixed by running
- ``archery lint --cmake-format --fix``. This requires Python
- 3 and `cmake_format <https://github.com/cheshirekow/cmake_format>`_ (note:
- this currently does not work on Windows).
+* Passes various C++ (and others) style checks by running
+ ``pre-commit run --show-diff-on-failure --color=always --all-files
+ cpp``.
On pull requests, the "Dev / Lint" pipeline will run these checks, and report
what files/lines need to be fixed, if any.
-In order to account for variations in the behavior of ``clang-format`` between
-major versions of LLVM, we pin the version of ``clang-format`` used. You can
-confirm the current pinned version by finding the ``CLANG_TOOLS`` variable
-value in `.env <https://github.com/apache/arrow/blob/main/.env>`_. Note that
-the version must match exactly; a newer version (even a patch release) will
-not work. LLVM can be installed through a system package manager or a package
-manager like Conda or Homebrew, though note they may not offer the exact
-version needed. Alternatively, binaries can be directly downloaded from the
-`LLVM website <https://releases.llvm.org/>`_.
Review Comment:
pre-commit installs pined clang-format automatically.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2138113390
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
-
-```r
-lintr::lint_package("arrow/r")
+```bash
+pre-commit run --show-diff-on-failure --color=always --all-files r
```
-You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
-
-Run the styler locally either via Makefile commands:
+See also the following subsections our styling and lint details for R and C++ codes.
-```bash
-make style # (for only the files changed)
-make style-all # (for all files)
-```
+### R code
-or in R:
+The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
-```r
-# note the file that should not be styled
-styler::style_pkg(exclude_files = c("data-raw/codegen.R"))
-```
+You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
The styler package will fix many styling errors, thought not all lintr errors are automatically fixable with styler. The list of files we intentionally do not style is in `r/.styler_excludes.R`.
Review Comment:
```suggestion
The styler package will fix many styling errors, thought not all lintr errors are automatically fixable with styler. The list of files we intentionally do not style is in `r/.styler_excludes.R`.
Linting and styling with [pre-commit](https://pre-commit.com/) as described above is the best way to ensure your changes are being checked properly but you can also run the tools individually if you prefer:
```r
lintr::lint_package() # for linting
styler::style_pkg() # for styling
```
Note: To run lintr, we require the `cyclocomp` package to be installed first.
```
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2122813044
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
-
-```r
-lintr::lint_package("arrow/r")
+```bash
+pre-commit run --show-diff-on-failure --color=always --all-files r
```
-You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
-
-Run the styler locally either via Makefile commands:
+See also the following subsections our styling and lint details for R and C++ codes.
-```bash
-make style # (for only the files changed)
-make style-all # (for all files)
-```
+### R code
-or in R:
+The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
-```r
-# note the file that should not be styled
-styler::style_pkg(exclude_files = c("data-raw/codegen.R"))
-```
+You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
Review Comment:
We'll use Air in the future.
It's tracked by https://github.com/apache/arrow/issues/46592 .
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2129177169
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
-
-```r
-lintr::lint_package("arrow/r")
+```bash
+pre-commit run --show-diff-on-failure --color=always --all-files r
```
-You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
Review Comment:
Let's leave this in too
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2143422007
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
> Could you share pre-commit related issues you got? Can we fix them instead of avoiding pre-commit?
Yeah, absolutely. I think it boils down to the fact that I had an older version - just updating it now to see if that fixes it. Maybe we wanna specify a minimum?
```
nic@xps-15:~$ cd arrow
nic@xps-15:~/arrow$ pre-commit
[INFO] Initializing environment for https://github.com/hadolint/hadolint.
[INFO] Initializing environment for https://github.com/pre-commit/mirrors-clang-format.
[INFO] Initializing environment for https://github.com/cpplint/cpplint.
[INFO] Initializing environment for https://github.com/hhatto/autopep8.
[INFO] Initializing environment for https://github.com/pycqa/flake8.
[INFO] Initializing environment for https://github.com/MarcoGorelli/cython-lint.
[INFO] Initializing environment for https://github.com/numpy/numpydoc.
[INFO] Initializing environment for local:cyclocomp,lintr.
[INFO] Initializing environment for local:roxygen2,styler.
[INFO] Initializing environment for https://github.com/rubocop/rubocop.
[INFO] Initializing environment for https://github.com/cheshirekow/cmake-format-precommit.
[INFO] Initializing environment for https://github.com/sphinx-contrib/sphinx-lint.
[INFO] Initializing environment for https://github.com/koalaman/shellcheck-precommit.
[INFO] Initializing environment for https://github.com/scop/pre-commit-shfmt.
An error has occurred: InvalidManifestError:
==> File /home/nic/.cache/pre-commit/repoepuluybh/.pre-commit-hooks.yaml
==> At Hook(id='shfmt')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit'
Check the log at /home/nic/.cache/pre-commit/pre-commit.log
nic@xps-15:~/arrow$ cat /home/nic/.cache/pre-commit/pre-commit.log
### version information
pre-commit version: 2.17.0
git --version: git version 2.34.1
sys.version:
3.10.12 (main, Feb 4 2025, 14:57:36) [GCC 11.4.0]
sys.executable: /usr/bin/python3
os.name: posix
sys.platform: linux
### error information
An error has occurred: InvalidManifestError:
==> File /home/nic/.cache/pre-commit/repoepuluybh/.pre-commit-hooks.yaml
==> At Hook(id='shfmt')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit'
Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/pre_commit/error_handler.py", line 70, in error_handler
yield
File "/usr/lib/python3/dist-packages/pre_commit/main.py", line 396, in main
return run(args.config, store, args)
File "/usr/lib/python3/dist-packages/pre_commit/commands/run.py", line 403, in run
for hook in all_hooks(config, store)
File "/usr/lib/python3/dist-packages/pre_commit/repository.py", line 228, in all_hooks
return tuple(
File "/usr/lib/python3/dist-packages/pre_commit/repository.py", line 231, in <genexpr>
for hook in _repository_hooks(repo, store, root_config)
File "/usr/lib/python3/dist-packages/pre_commit/repository.py", line 206, in _repository_hooks
return _cloned_repository_hooks(repo_config, store, root_config)
File "/usr/lib/python3/dist-packages/pre_commit/repository.py", line 173, in _cloned_repository_hooks
by_id = {hook['id']: hook for hook in load_manifest(manifest_path)}
File "/usr/lib/python3/dist-packages/cfgv.py", line 394, in load_from_filename
with reraise_as(exc_tp):
File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
self.gen.throw(typ, value, traceback)
File "/usr/lib/python3/dist-packages/cfgv.py", line 43, in reraise_as
raise tp(e).with_traceback(tb) from None
File "/usr/lib/python3/dist-packages/cfgv.py", line 40, in reraise_as
yield
File "/usr/lib/python3/dist-packages/cfgv.py", line 398, in load_from_filename
with validate_context(f'File {filename}'):
File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
self.gen.throw(typ, value, traceback)
File "/usr/lib/python3/dist-packages/cfgv.py", line 34, in validate_context
raise ValidationError(e, ctx=msg).with_traceback(tb) from None
File "/usr/lib/python3/dist-packages/cfgv.py", line 31, in validate_context
yield
File "/usr/lib/python3/dist-packages/cfgv.py", line 410, in load_from_filename
validate(data, schema)
File "/usr/lib/python3/dist-packages/cfgv.py", line 376, in validate
schema.check(v)
File "/usr/lib/python3/dist-packages/cfgv.py", line 272, in check
validate(val, self.of)
File "/usr/lib/python3/dist-packages/cfgv.py", line 376, in validate
schema.check(v)
File "/usr/lib/python3/dist-packages/cfgv.py", line 242, in check
with validate_context(context):
File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
self.gen.throw(typ, value, traceback)
File "/usr/lib/python3/dist-packages/cfgv.py", line 34, in validate_context
raise ValidationError(e, ctx=msg).with_traceback(tb) from None
File "/usr/lib/python3/dist-packages/cfgv.py", line 31, in validate_context
yield
File "/usr/lib/python3/dist-packages/cfgv.py", line 244, in check
item.check(v)
File "/usr/lib/python3/dist-packages/cfgv.py", line 53, in _check_optional
with validate_context(f'At key: {self.key}'):
File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
self.gen.throw(typ, value, traceback)
File "/usr/lib/python3/dist-packages/cfgv.py", line 34, in validate_context
raise ValidationError(e, ctx=msg).with_traceback(tb) from None
File "/usr/lib/python3/dist-packages/cfgv.py", line 31, in validate_context
yield
File "/usr/lib/python3/dist-packages/cfgv.py", line 54, in _check_optional
self.check_fn(dct[self.key])
File "/usr/lib/python3/dist-packages/cfgv.py", line 363, in check_array_fn
with validate_context(f'At index {i}'):
File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
self.gen.throw(typ, value, traceback)
File "/usr/lib/python3/dist-packages/cfgv.py", line 34, in validate_context
raise ValidationError(e, ctx=msg).with_traceback(tb) from None
File "/usr/lib/python3/dist-packages/cfgv.py", line 31, in validate_context
yield
File "/usr/lib/python3/dist-packages/cfgv.py", line 364, in check_array_fn
inner_check(val)
File "/usr/lib/python3/dist-packages/cfgv.py", line 342, in check_one_of_fn
raise ValidationError(
pre_commit.clientlib.InvalidManifestError:
==> File /home/nic/.cache/pre-commit/repoepuluybh/.pre-commit-hooks.yaml
==> At Hook(id='shfmt')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit'
```
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2144320145
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
I feel like I did get some error when I ran the full command but can't recall now as I've just updated pre-commit. Thanks @kou! :)
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2133132567
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
Or is running `lintr::lint_package("arrow/r")` directly better?
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2138745468
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
Thanks for your 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2131321873
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
OK.
Is it OK that running `r/tools/lint.R` for non pre-commit option? R developers need to install `lintr` manually with this approach.
(Can we remove `ci/docker/linux-apt-lint.dockerfile`? I want to remove `ci/docker/linux-apt-lint.dockerfile`.)
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #46686:
URL: https://github.com/apache/arrow/pull/46686#issuecomment-2964743356
I'll merge this in a few days if nobody objects 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #46686:
URL: https://github.com/apache/arrow/pull/46686#issuecomment-2934057896
Good catch! We should remove them. I'll do 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2138113390
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
-
-```r
-lintr::lint_package("arrow/r")
+```bash
+pre-commit run --show-diff-on-failure --color=always --all-files r
```
-You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
-
-Run the styler locally either via Makefile commands:
+See also the following subsections our styling and lint details for R and C++ codes.
-```bash
-make style # (for only the files changed)
-make style-all # (for all files)
-```
+### R code
-or in R:
+The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
-```r
-# note the file that should not be styled
-styler::style_pkg(exclude_files = c("data-raw/codegen.R"))
-```
+You can automatically change the formatting of the code in the package using the [styler](https://styler.r-lib.org/) package.
The styler package will fix many styling errors, thought not all lintr errors are automatically fixable with styler. The list of files we intentionally do not style is in `r/.styler_excludes.R`.
Review Comment:
````suggestion
The styler package will fix many styling errors, thought not all lintr errors are automatically fixable with styler. The list of files we intentionally do not style is in `r/.styler_excludes.R`.
Linting and styling with [pre-commit](https://pre-commit.com/) as described above is the best way to ensure your changes are being checked properly but you can also run the tools individually if you prefer:
```r
lintr::lint_package() # for linting
styler::style_pkg() # for styling
```
Note: To run lintr, we require the `cyclocomp` package to be installed first.
````
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2129176670
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
Thanks for making this all a lot more concise!
We should leave in information about how to run it other than with pre-commit. Many R users are more comfortable with running `lintr` directly as it's much simpler (I've personally had issues getting pre-commit set up so won't be the only one), so it'd be great to give them both options.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2138087415
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
Review Comment:
```suggestion
Styling and linting can be set up and performed entirely with the [pre-commit](https://pre-commit.com/) tool:
```
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2144028933
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
Thanks. You're using Ubuntu 22.04, right?
How about the following?
```diff
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index c3835ac0f1..e3e8abdeed 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -356,7 +356,10 @@ repos:
?^swift/gen-protobuffers\.sh$|
)
- repo: https://github.com/scop/pre-commit-shfmt
- rev: v3.11.0-1
+ # v3.11.0-1 or later requires pre-commit 3.2.0 or later but Ubuntu
+ # 22.04 ships pre-commit 2.17.0. We can use update this rev after
+ # Ubuntu 22.04 reached EOL (June 2027).
+ rev: v3.10.0-1
hooks:
- id: shfmt
alias: shell
```
I've opened #46798 for this.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2136698233
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
OK. Let's use `lintr::lint_package("arrow/r")`.
> the core dev team generally runs `arrow/r/lint.sh` to lint R and C++ at the same time.
Hmm... This PR removes `r/lint.sh`...
Could you share pre-commit related issues you got? Can we fix them instead of avoiding pre-commit?
> I'm not sure we can remove linux-apt-lint.dockerfile just yet. IIRC the way that's set up, it will catch lints in the R packages internal C++ code but the current `r.yml` workflow just lints the R code so removing linux-apt-lint.dockerfile could cause us to merge R C++ code with lint issues. I think we could modify the R workflow to keep the functionality though.
Oh, I missed that the `r.yml` workflow also runs lintr: https://github.com/apache/arrow/blob/b630f48f8464e770341e053e2fb328bd857bf72e/.github/workflows/r.yml#L334-L351
(BTW, I didn't know that `lintr::expect_lint_free()` exists. I changed the pre-commit configuration to use `lintr::expect_lint_free()` and removes `r/tools/lint.R`.)
I want to remove the "Run lintr" step too because it's already done by pre-commit https://github.com/apache/arrow/blob/b630f48f8464e770341e053e2fb328bd857bf72e/.github/workflows/dev.yml#L66-L68 and `archery lint` https://github.com/apache/arrow/blob/b630f48f8464e770341e053e2fb328bd857bf72e/.github/workflows/dev.yml#L71-L78 .
And pre-commit and `archery lint` check not only R code but also R C++ code. So we will not merge R C++ code with lint issues.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2136057930
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
Running lintr::lint_package("arrow/r") is better for a non-pre-commit option, yes. `arrow/r/lint.R` could possibly be removed once it's unhooked from CI. The average R contributor will probably run lintr::lint_package to lint and the core dev team generally runs `arrow/r/lint.sh` to lint R and C++ at the same time.
I'm not sure we can remove linux-apt-lint.dockerfile just yet. IIRC the way that's set up, it will catch lints in the R packages internal C++ code but the current `r.yml` workflow just lints the R code so removing linux-apt-lint.dockerfile could cause us to merge R C++ code with lint issues. I think we could modify the R workflow to keep the functionality though. Happy to work on that.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2144031268
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
FYI: You can use `pre-commit run --all-files r` only for R as described in the changed document.
In the case, shfmt isn't used. So the error isn't occurred.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #46686:
URL: https://github.com/apache/arrow/pull/46686
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "kszucs (via GitHub)" <gi...@apache.org>.
kszucs commented on PR #46686:
URL: https://github.com/apache/arrow/pull/46686#issuecomment-2945110019
We have duplicated most of the linters in both archery and pre-commit hooks also causing version and configuration mismatches for certain tools, so I strongly support this change.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2138121661
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
I added a couple of suggestions, including one to this effect.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #46686:
URL: https://github.com/apache/arrow/pull/46686#discussion_r2138069655
##########
r/vignettes/developers/workflow.Rmd:
##########
@@ -51,31 +51,19 @@ pkgdown::build_site(preview=TRUE)
## Styling and linting
-### R code
-
-The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
+You can automatically adjust our styling and lint by [pre-commit](https://pre-commit.com/):
-To run the linter locally, install the `{lintr}` package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file [`ci/docker/linux-apt-lint.dockerfile`](https://github.com/apache/arrow/blob/main/ci/docker/linux-apt-lint.dockerfile) for the current status) and then run
Review Comment:
Okay. I just tested locally with the changes in your PR and I do now see that both R and C++ code in the R package are checked and formatted. Nice! So +1 (remove Run lintr step, remove linux-apt-lint.dockerfile).
I do agree with @thisisnic on keeping standalone instructions at least for lintr.
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #46686:
URL: https://github.com/apache/arrow/pull/46686#issuecomment-2938798863
That's fine with me. @kszucs Any comments?
--
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-46528: [CI][Dev] Remove "archery lint" [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #46686:
URL: https://github.com/apache/arrow/pull/46686#issuecomment-2970770404
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit c314e80eec38684d3080694e5c1d3b5a3089e219.
There were 73 benchmark results with an error:
- Commit Run on `arm64-t4g-2xlarge-linux` at [2025-06-13 08:54:39Z](https://conbench.ursa.dev/compare/runs/d3bd90b6893c4114bc78f15cf3dc07e1...c0018a13ce3746a4b9289ffb22f67610/)
- [`tpch` (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-07, scale_factor=1](https://conbench.ursa.dev/benchmark-results/0684bfa0f8b1725e8000f61d6bb01492)
- [`tpch` (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-09, scale_factor=1](https://conbench.ursa.dev/benchmark-results/0684bfac47d07e3680009e6edd6b439f)
- and 71 more (see the report linked below)
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/44059317064) has more details. It also includes information about 2 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