Skip to content

chore: Use only pre-commit hooks #2057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .githooks/pre-commit

This file was deleted.

33 changes: 0 additions & 33 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand Down
63 changes: 54 additions & 9 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
25 changes: 13 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 <your changed files>` for C++ files and `cmake-format -i <your changed files>` 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 <your changed files>` for C++ files, and `pre-commit run cmake-format --files <your changed files>` for CMake files.

### Documentation

Expand Down
2 changes: 1 addition & 1 deletion docker/ci/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
File renamed without changes.
23 changes: 4 additions & 19 deletions .githooks/check-format → pre-commit-hooks/fix-local-includes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.

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
# This script checks will fix local includes in the C++ code.

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
}
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
3 changes: 3 additions & 0 deletions pre-commit-hooks/lfs/pre-push
Original file line number Diff line number Diff line change
@@ -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 "$@"
4 changes: 0 additions & 4 deletions .githooks/pre-push → pre-commit-hooks/verify-commits.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$@"
Loading