You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "jorisvandenbossche (via GitHub)" <gi...@apache.org> on 2026/08/11 14:01:42 UTC

[PR] GH-47937: [dev][R] Replace linr with jarl for R linting / pre-commit … [arrow]

jorisvandenbossche opened a new pull request, #50851:
URL: https://github.com/apache/arrow/pull/50851

   …check
   
   ### Rationale for this change
   
   Fix #46646 by replacing the `lintr` pre-commit hook (which requires system R/Rscript to bootstrap, causing hard failures when R isn't installed, end even if installed still gives errors when using conda (https://github.com/apache/arrow/issues/47997)) with jarl (https://jarl.etiennebacher.com/, a Rust-native R linter distributed as a standalone binary or as a python package).
   
   ### What changes are included in this PR?
   
   - `.pre-commit-config.yaml`: swapped the local `lintr` hook for `etiennebacher/jarl-pre-commit` (jarl-check, rev 0.5.0)
   - `r/jarl.toml`: new config file, excludes the same generated files that `r/.lintr` and `r/air.toml` already exclude
     - NOTE: for now I did not remove `r/.lintr` in case people would use that locally or through their IDE, so it might be useful to keep. But for our CI it could be removed.
   - `.github/workflows/dev.yml`: dropped r-base from the CI apt install and the now-unused `~/.local/share/renv/cache` from the pre-commit cache path, since no pre-commit hook needs R anymore
   - `r/vignettes/developers/workflow.Rmd` and `docs/source/developers/guide/step_by_step/styling.rst`: updated references from lintr to jarl
   
   At the moment, running `jarl check` gives 42 pre-existing errors (13 auto-fixable), so CI will be failing at the moment. 
   Those should be fixed (or temporarily suppressed) in a next commit.
   
   Also note the differences between lintr and jarl listed at https://github.com/apache/arrow/issues/46646#issuecomment-5253845795. Especially no cyclocomp (complexity) or object_length equivalent exists in jarl, so we have to see if we are fine with dropping those checks.
   
   Code changes are largely generated by Claude, I did review everything, but I am no R developer ..
   
   ### Are these changes tested?
   
   pre-commit runs on CI
   
   ### 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-47937: [dev][R] Replace linr with jarl for R linting / pre-commit check [arrow]

Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #50851:
URL: https://github.com/apache/arrow/pull/50851#discussion_r3758780035


##########
.pre-commit-config.yaml:
##########
@@ -188,19 +188,12 @@ repos:
           ?^python/pyarrow/util\.py$|
           ?^python/pyarrow/vendored/|
           )
-  - repo: local
+  - repo: https://github.com/etiennebacher/jarl-pre-commit
+    rev: "0.5.0"
     hooks:
-      - id: lintr
+      - id: jarl-check
         alias: r
         name: R Lint
-        language: r
-        additional_dependencies:
-          - cyclocomp
-          - lintr
-          - testthat
-        entry: |
-          Rscript -e "Sys.setenv(NOT_CRAN = 'TRUE'); lintr::expect_lint_free('r')"
-        pass_filenames: false
         files: >-
           ^r/.*\.(R|Rmd)$

Review Comment:
   The pre-commit logs in the failing CI clearly indicates `Used '/home/runner/work/arrow/arrow/r/jarl.toml'`, so it seems the config file _is_ correctly used



-- 
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-46646: [dev][R] Replace linr with jarl for R linting / pre-commit check [arrow]

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #50851:
URL: https://github.com/apache/arrow/pull/50851


-- 
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-46646: [dev][R] Replace linr with jarl for R linting / pre-commit check [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #50851:
URL: https://github.com/apache/arrow/pull/50851#issuecomment-5254586600

   :warning: GitHub issue #46646 **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


Re: [PR] GH-47937: [dev][R] Replace linr with jarl for R linting / pre-commit check [arrow]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50851:
URL: https://github.com/apache/arrow/pull/50851#discussion_r3758711664


##########
.pre-commit-config.yaml:
##########
@@ -188,19 +188,12 @@ repos:
           ?^python/pyarrow/util\.py$|
           ?^python/pyarrow/vendored/|
           )
-  - repo: local
+  - repo: https://github.com/etiennebacher/jarl-pre-commit
+    rev: "0.5.0"
     hooks:
-      - id: lintr
+      - id: jarl-check
         alias: r
         name: R Lint
-        language: r
-        additional_dependencies:
-          - cyclocomp
-          - lintr
-          - testthat
-        entry: |
-          Rscript -e "Sys.setenv(NOT_CRAN = 'TRUE'); lintr::expect_lint_free('r')"
-        pass_filenames: false
         files: >-
           ^r/.*\.(R|Rmd)$

Review Comment:
   `jarl.toml` is added under `r/`, but the pre-commit hook runs from the repo root. Per Jarl’s config discovery, it only searches `jarl.toml` in the current working directory and parent directories, so `r/jarl.toml` won’t be picked up; the excludes there won’t apply and generated files may get linted unexpectedly. Consider either moving `jarl.toml` to the repo root, or changing the hook to execute with `cwd=r/` (bearing in mind Windows support / filename path rewriting).



-- 
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-46646: [dev][R] Replace linr with jarl for R linting / pre-commit check [arrow]

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #50851:
URL: https://github.com/apache/arrow/pull/50851#issuecomment-5370717743

   After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 71c98fcbd13d5834bc1338c11d2d6ac6cfd540e1.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/96793114125) has more details.


-- 
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-46646: [dev][R] Replace linr with jarl for R linting / pre-commit check [arrow]

Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #50851:
URL: https://github.com/apache/arrow/pull/50851#issuecomment-5367329083

   > the pre-commit stuff is annoying (I just turn it off most of the time when developing locally)
   
   FWIW, my personal stance is that pre-commit in the first place is there to help the local developer. So if you are turning it off locally because it is annoying, we should fix it (on CI we can easily run the validation steps differently).
   
   One option is to still include those steps that are annoying locally, but have them not run by default and only run those on CI (we can mark those hooks as manual with `stages: [manual]`, and then run those explicitly on CI). 
   
   For example we could keep the lintr cyclomatic complexity feature as a manual stage that is still run on CI (and we can still document the command for contributors to also run that locally).
   
   


-- 
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-47937: [dev][R] Replace linr with jarl for R linting / pre-commit … [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #50851:
URL: https://github.com/apache/arrow/pull/50851#issuecomment-5254206914

   :warning: GitHub issue #47937 **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