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-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to