diff --git a/.githooks/pre-commit b/.githooks/pre-commit deleted file mode 100755 index 87d6b66f..00000000 --- a/.githooks/pre-commit +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash - -# This script is intended to be run from the root of the repository. - -source .githooks/check-format -source .githooks/check-docs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a615af35..681ef2bb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,41 +7,8 @@ on: workflow_dispatch: jobs: - check_format: - name: Check format - runs-on: ubuntu-latest - container: - image: ghcr.io/xrplf/clio-ci:latest - steps: - - name: Fix git permissions on Linux - shell: bash - run: git config --global --add safe.directory $PWD - - - uses: actions/checkout@v4 - - name: Run formatters - id: run_formatters - run: | - ./.githooks/check-format --diff - shell: bash - - check_docs: - name: Check documentation - runs-on: ubuntu-latest - container: - image: ghcr.io/xrplf/clio-ci:latest - steps: - - uses: actions/checkout@v4 - - name: Run linter - id: run_linter - run: | - ./.githooks/check-docs - shell: bash - build: name: Build - needs: - - check_format - - check_docs strategy: fail-fast: false matrix: diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 89ff9295..f7f88f85 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -65,11 +65,11 @@ jobs: run: | run-clang-tidy-19 -p build -j ${{ steps.number_of_threads.outputs.threads_number }} -fix -quiet 1>output.txt - - name: Check format + - name: Fix local includes if: ${{ steps.run_clang_tidy.outcome != 'success' }} continue-on-error: true shell: bash - run: ./.githooks/check-format + run: pre-commit run --all-files fix-local-includes - name: Print issues found if: ${{ steps.run_clang_tidy.outcome != 'success' }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c0d44410..210905a9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -31,15 +31,6 @@ repos: - id: prettier exclude: ^docs/doxygen-awesome-theme/ - - repo: local - hooks: - - id: gofmt - name: Go Format - entry: pre-commit-hooks/run-go-fmt.sh - types: [go] - language: golang - description: "Runs `gofmt`, requires golang" - - repo: https://github.com/igorshubovych/markdownlint-cli rev: v0.44.0 hooks: @@ -71,3 +62,57 @@ repos: entry: There should be no .h files in this repository language: fail files: \.h$ + + - repo: local + hooks: + - id: gofmt + name: Go Format + entry: pre-commit-hooks/run-go-fmt.sh + types: [go] + language: golang + description: "Runs `gofmt`, requires golang" + - id: check-docs + name: Check Doxygen Documentation + entry: pre-commit-hooks/check-doxygen-docs.sh + types: [text] + language: system + pass_filenames: false + - id: fix-local-includes + name: Fix Local Includes + entry: pre-commit-hooks/fix-local-includes.sh + types: [c++] + language: system + pass_filenames: false + - id: verify-commits + name: Verify Commits + entry: pre-commit-hooks/verify-commits.sh + types: [text] + language: system + pass_filenames: false + + - repo: local + hooks: + - id: lfs-post-checkout + name: LFS Post Checkout + entry: pre-commit-hooks/lfs/post-checkout + types: [text] + stages: [post-checkout] + language: system + - id: lfs-post-commit + name: LFS Post Commit + entry: pre-commit-hooks/lfs/post-commit + types: [text] + stages: [post-commit] + language: system + - id: lfs-post-merge + name: LFS Post Merge + entry: pre-commit-hooks/lfs/post-merge + types: [text] + stages: [post-merge] + language: system + - id: lfs-pre-push + name: LFS Pre Push + entry: pre-commit-hooks/lfs/pre-push + types: [text] + stages: [pre-push] + language: system diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 72f0e5e6..ed781906 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,26 +11,27 @@ To contribute, please: 3. Write and test your code. 4. Ensure that your code compiles with the provided build engine and update the provided build engine as part of your PR where needed and where appropriate. 5. Where applicable, write test cases for your code and include those in the relevant subfolder under `tests`. -6. Ensure your code passes automated checks (e.g. clang-format) +6. Ensure your code passes [automated checks](#pre-commit-hooks) 7. Squash your commits (i.e. rebase) into as few commits as is reasonable to describe your changes at a high level (typically a single commit for a small change). See below for more details. 8. Open a PR to the main repository onto the _develop_ branch, and follow the provided template. > **Note:** Please read the [Style guide](#style-guide). -### Install git hooks +### `pre-commit` hooks -Please run the following command in order to use git hooks that are helpful for `clio` development. +To ensure code quality and style, we use [`pre-commit`](https://pre-commit.com/). + +Run the following command to enable `pre-commit` hooks that help with Clio development: ```bash -git config --local core.hooksPath .githooks +pip3 install pre-commit +pre-commit install ``` -### Git hooks dependencies +`pre-commit` takes care of running each tool in [`.pre-commit-config.yaml`](https://github.com/XRPLF/clio/blob/develop/.pre-commit-config.yaml) in a separate environment. -The pre-commit hook requires `clang-format >= 19.0.0` and `cmake-format` to be installed on your machine. -`clang-format` can be installed using `brew` on macOS and default package manager on Linux. -`cmake-format` can be installed using `pip`. -The hook will also attempt to automatically use `doxygen` to verify that everything public in the codebase is covered by doc comments. If `doxygen` is not installed, the hook will raise a warning suggesting to install `doxygen` for future commits. +`pre-commit` also attempts to automatically use Doxygen to verify that everything public in the codebase has doc comments. +If Doxygen is not installed, the hook issues a warning and recommends installing Doxygen for future commits. ### Git commands @@ -120,9 +121,9 @@ This is a non-exhaustive list of recommended style guidelines. These are not alw ### Formatting -Code must conform to `clang-format` version 19, unless the result would be unreasonably difficult to read or maintain. -In most cases the pre-commit hook will take care of formatting and will fix any issues automatically. -To manually format your code, use `clang-format -i ` for C++ files and `cmake-format -i ` for CMake files. +Code must conform to `clang-format`, unless the result is unreasonably difficult to read or maintain. +In most cases the `pre-commit` hook takes care of formatting and fixes any issues automatically. +To manually format your code, run `pre-commit run clang-format --files ` for C++ files, and `pre-commit run cmake-format --files ` for CMake files. ### Documentation diff --git a/docker/ci/Dockerfile b/docker/ci/Dockerfile index 2bbf5d96..de3a2aad 100644 --- a/docker/ci/Dockerfile +++ b/docker/ci/Dockerfile @@ -21,7 +21,7 @@ RUN apt-get -qq update \ RUN apt update -qq \ && apt install -y --no-install-recommends --no-install-suggests python3 python3-pip git git-lfs make ninja-build flex bison jq graphviz \ clang-tidy-${LLVM_TOOLS_VERSION} clang-tools-${LLVM_TOOLS_VERSION} \ - && pip3 install -q --upgrade --no-cache-dir pip && pip3 install -q --no-cache-dir conan==1.62 gcovr cmake==3.31.6 \ + && pip3 install -q --upgrade --no-cache-dir pip && pip3 install -q --no-cache-dir conan==1.62 gcovr cmake==3.31.6 pre-commit \ && apt-get clean && apt remove -y software-properties-common # Install gcc-12 and make ldconfig aware of the new libstdc++ location (for gcc) diff --git a/.githooks/check-docs b/pre-commit-hooks/check-doxygen-docs.sh similarity index 100% rename from .githooks/check-docs rename to pre-commit-hooks/check-doxygen-docs.sh diff --git a/.githooks/check-format b/pre-commit-hooks/fix-local-includes.sh similarity index 69% rename from .githooks/check-format rename to pre-commit-hooks/fix-local-includes.sh index a8a0c87c..e4f7ce1c 100755 --- a/.githooks/check-format +++ b/pre-commit-hooks/fix-local-includes.sh @@ -2,28 +2,13 @@ # Note: This script is intended to be run from the root of the repository. # -# This script checks the format of the C++ code. -# In many cases it will automatically fix the issues and abort the commit. +# This script checks will fix local includes in the C++ code. -no_formatted_directories_staged() { - staged_directories=$(git diff-index --cached --name-only HEAD | awk -F/ '{print $1}') - for sd in $staged_directories; do - if [[ "$sd" =~ ^(src|tests)$ ]]; then - return 1 - fi - done - return 0 -} - -if no_formatted_directories_staged ; then - exit 0 -fi - -echo "+ Checking code format..." - -# paths to check and re-format +# paths to fix include statements sources="src tests" +echo "+ Fixing local includes..." + function grep_code { grep -l "${1}" ${sources} -r --include \*.hpp --include \*.cpp } diff --git a/.githooks/post-checkout b/pre-commit-hooks/lfs/post-checkout similarity index 100% rename from .githooks/post-checkout rename to pre-commit-hooks/lfs/post-checkout diff --git a/.githooks/post-commit b/pre-commit-hooks/lfs/post-commit similarity index 100% rename from .githooks/post-commit rename to pre-commit-hooks/lfs/post-commit diff --git a/.githooks/post-merge b/pre-commit-hooks/lfs/post-merge similarity index 100% rename from .githooks/post-merge rename to pre-commit-hooks/lfs/post-merge diff --git a/pre-commit-hooks/lfs/pre-push b/pre-commit-hooks/lfs/pre-push new file mode 100755 index 00000000..0f0089bc --- /dev/null +++ b/pre-commit-hooks/lfs/pre-push @@ -0,0 +1,3 @@ +#!/bin/sh +command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks').\n"; exit 2; } +git lfs pre-push "$@" diff --git a/.githooks/pre-push b/pre-commit-hooks/verify-commits.sh similarity index 82% rename from .githooks/pre-push rename to pre-commit-hooks/verify-commits.sh index 8d4f51a6..a224aa90 100755 --- a/.githooks/pre-push +++ b/pre-commit-hooks/verify-commits.sh @@ -52,7 +52,3 @@ while read local_ref local_oid remote_ref remote_oid; do fi fi done - -command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks').\n"; exit 2; } - -git lfs pre-push "$@"