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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
