Skip to content
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

Add an option to apply pre-commit and clang-tidy fixes #238

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
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
16 changes: 15 additions & 1 deletion .github/workflows/build_doc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,26 @@ jobs:
with:
folder: docs_build
target-folder: latest

- name: Find Comment
uses: peter-evans/find-comment@v1
id: fc
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: 'github-actions[bot]'

- name: Comment PR
uses: thollander/actions-comment-pull-request@v2
if: steps.fc.outputs.comment-id == ''
with:
message: |
Deployed test documentation to https://exasim-project.com/NeoFOAM/Build_PR_${{ env.PR_NUMBER }}
Thank you for your PR, here are some useful tips:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bot seems a bit spammy. Is there a way to only post once per PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should actually post it only once. I just deleted the comments of the bot to check how the most recent version renders.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's removing the old comment and adding a new one. I still got multiple emails with the same bot comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this action should use the pull_request:open event. That would probably mean to extract it into a new file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok this should be resolved now with a999de0
i just check if the bot already posted.

- Use labels to change the workflow behavior. Set `auto-fix` to automatically apply fixes, `full-ci` for a comprehensive set of tests and sanitizer runs, `ready-for-review` to show that you're done and need a review.
- I deployed a test documentation to https://exasim-project.com/NeoFOAM/Build_PR_${{ env.PR_NUMBER }}
- For `full-ci` builds benchmark results are available at https://github.com/exasim-project/NeoFOAM-BenchmarkData/${{ env.PR_NUMBER }}
- If this is your first PR, please add yourself to the AUTHORS.md file
comment_tag: build_url
mode: createIfNotExists

- name: Echo Build PR URL
run: |
Expand Down
69 changes: 47 additions & 22 deletions .github/workflows/static_checks.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,45 @@
name: Static checks
run-name: Static checks

on:
pull_request:
types: [opened, synchronize]
types: [opened,synchronize,labeled]
env:
BRANCH_NAME: ${{ github.head_ref || github.ref_name }}
permissions:
contents: write
jobs:
pre-commit-run:
name: Pre-commit run
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{env.BRANCH_NAME}}
- name: Install dependencies
run: |
sudo apt update
sudo apt install \
pre-commit
pip install identify --upgrade
pre-commit install
- name: Run pre-commit on all files
id: pre-commit
run: pre-commit run --all --color always --verbose
- name: Commit pre-commit fixes
if: ${{contains(github.event.pull_request.labels.*.name, 'auto-fix') && failure() && steps.pre-commit.conclusion == 'failure'}}
run: |
git config user.name github-actions
git config user.email [email protected]
git add .
git commit -m 'fix format'
git push origin ${{env.BRANCH_NAME}}
- name: check for todo fixme note
run: |
NTODOS="$(grep -r 'TODO DONT MERGE' --exclude-dir=.github | wc -l)"
echo "Found $NTODOS TODO DONT MERGE notes"
! grep -q -r "TODO DONT MERGE" --exclude-dir=.github
build-compilation-db:
if: ${{!contains(github.event.pull_request.labels.*.name, 'Skip-build')}}
if: ${{!contains(github.event.pull_request.labels.*.name, 'Skip-build') }}
name: Build with IWYU
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -55,6 +88,8 @@ jobs:
needs: [build-compilation-db]
steps:
- uses: actions/checkout@v4
with:
ref: ${{env.BRANCH_NAME}}

- name: Add clang repo
run: |
Expand Down Expand Up @@ -88,8 +123,8 @@ jobs:
| grep -F -f pattern - > files
# run clang-tidy on all specified files
clang-tidy-16 --fix --extra-arg=-w -p build/develop $(cat files)

- name: Check for fixes
id: tidy-fixes
run: |
if [[ $(git ls-files -m | wc -l) -eq 0 ]]; then
exit 0
Expand All @@ -98,25 +133,15 @@ jobs:
echo "Please use your local clang-tidy or apply the following patch:"
git diff -p
exit 1
pre-commit-run:
name: Pre-commit run
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install dependencies
- name: Commit clang-tidy-fixes
if: ${{contains(github.event.pull_request.labels.*.name, 'auto-fix') && failure() && steps.tidy-fixes.conclusion == 'failure'}}
run: |
sudo apt update
sudo apt install \
pre-commit
pip install identify --upgrade
pre-commit install
- name: Run pre-commit on all files
run: pre-commit run --all --color always --verbose
- name: check for todo fixme note
run: |
NTODOS="$(grep -r 'TODO DONT MERGE' --exclude-dir=.github | wc -l)"
echo "Found $NTODOS TODO DONT MERGE notes"
! grep -q -r "TODO DONT MERGE" --exclude-dir=.github
rm pattern files
git config user.name github-actions
git config user.email [email protected]
git add .
git commit -m 'fix format'
git push origin ${{ github.BRANCH_NAME }}
changelog:
if: ${{!contains(github.event.pull_request.labels.*.name, 'Skip-Changelog')}}
name: Changelog check
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,4 @@ NeoFOAM/*
_build

.vs/
*.key
3 changes: 2 additions & 1 deletion doc/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ The following labels and their meaning are discussed here:
* ``Skip-Changelog``: Don't check whether the changelog has been updated. Use this label if the changes are not any new features or bug-fixes.
* ``Skip-build``: Don't run any build steps, including building the compile commands database.
* ``Skip-cache``: Don't cache the build folders. Forces to rebuild the build folder after every push to GitHub.
* ``full-ci``: Run tests on AWS.
* ``full-ci``: Run tests on AWS and enable sanitizer workflows.
* ``auto-fix``: Apply changes from pre-commit and clang-tidy.

A full list of the labels can be found `here <https://github.com/exasim-project/NeoFOAM/labels>`_.

Expand Down
Loading