GH-46646: [dev][R] Replace linr with jarl for R linting / pre-commit check - #50851
Open
jorisvandenbossche wants to merge 2 commits into
Open
GH-46646: [dev][R] Replace linr with jarl for R linting / pre-commit check#50851jorisvandenbossche wants to merge 2 commits into
jorisvandenbossche wants to merge 2 commits into
Conversation
jorisvandenbossche
requested review from
assignUser,
jonkeane,
kou,
raulcd and
thisisnic
as code owners
August 11, 2026 14:01
|
|
jorisvandenbossche
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces the R linting pre-commit hook that depended on system R (lintr via Rscript) with the Rust-native jarl hook, and updates CI/docs accordingly to avoid hard failures when R isn’t installed.
Changes:
- Swap the local
lintrpre-commit hook foretiennebacher/jarl-pre-commit(jarl-check). - Add an initial
r/jarl.tomlconfiguration and update developer docs to referencejarlinstead oflintr. - Simplify the dev GitHub Actions lint job by removing
r-baseinstall and therenvcache path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| r/vignettes/developers/workflow.Rmd | Updates R developer workflow docs to use jarl instead of lintr. |
| r/jarl.toml | Adds Jarl configuration (notably excludes for generated files). |
| docs/source/developers/guide/step_by_step/styling.rst | Updates styling docs to reference jarl. |
| .pre-commit-config.yaml | Replaces the R lint hook implementation from lintr to jarl. |
| .github/workflows/dev.yml | Removes r-base and renv cache entries now that pre-commit no longer needs R. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
191
to
198
| - 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)$ |
Member
Author
There was a problem hiding this 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
github-actions
Bot
added
awaiting changes
github-actions
Bot
added
awaiting change review
jorisvandenbossche
changed the title
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Fix #46646 by replacing the
lintrpre-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 (#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 locallintrhook foretiennebacher/jarl-pre-commit(jarl-check, rev 0.5.0)r/jarl.toml: new config file, excludes the same generated files thatr/.lintrandr/air.tomlalready excluder/.lintrin 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/cachefrom the pre-commit cache path, since no pre-commit hook needs R anymorer/vignettes/developers/workflow.Rmdanddocs/source/developers/guide/step_by_step/styling.rst: updated references from lintr to jarlAt the moment, running
jarl checkgives 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. EDIT: for now I added those to the configuration as ignored rules, so CI would be green. Those can be fixed as a follow-up issue.
Also note the differences between lintr and jarl listed at #46646 (comment). 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