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

Conversation

greole
Copy link
Contributor

@greole greole commented Jan 13, 2025

This PR modifies the static-checks workflow to commit changes if the auto-fix label is set. For now, applying fixes automatically is not the default, because it might be annoying to constantly push and pull.

Additional changes:

  • changed the message of the github bot for more details.

@greole greole changed the title make it always succeed Add an option to apply pre-commit and clang-tidy fixes Jan 13, 2025
@exasim-project exasim-project deleted a comment from github-actions bot Jan 13, 2025
@exasim-project exasim-project deleted a comment from github-actions bot Jan 13, 2025
@exasim-project exasim-project deleted a comment from github-actions bot Jan 13, 2025
- name: Run pre-commit on all files
run: pre-commit run --all --color always --verbose
- name: Commit pre-commit fixes
if: ${{contains(github.event.pull_request.labels.*.name, 'auto-fix') && failure()}}
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 you can check for the failure state of a specific step. This would also trigger, if, for example, the install dependencies step failed.

@greole greole force-pushed the enh/formating_workflow branch from 40022fa to d79a82b Compare January 13, 2025 11:08
@exasim-project exasim-project deleted a comment from github-actions bot Jan 13, 2025
@greole greole force-pushed the enh/formating_workflow branch from 2e7dbb9 to 369f112 Compare January 13, 2025 11:20
@greole greole added the build Everything related to building NF label Jan 13, 2025
@greole greole force-pushed the enh/formating_workflow branch from b20cb49 to b6ed0c0 Compare January 13, 2025 11:48
@greole greole force-pushed the enh/formating_workflow branch from b6ed0c0 to 72b183e Compare January 13, 2025 12:33
@@ -56,7 +56,10 @@ jobs:
uses: thollander/actions-comment-pull-request@v2
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.

@greole greole requested a review from MarcelKoch January 13, 2025 12:51
@greole greole force-pushed the enh/formating_workflow branch from 0d1a162 to d437b4d Compare January 13, 2025 13:10
@exasim-project exasim-project deleted a comment from github-actions bot Jan 14, 2025
Copy link

Thank you for your PR, here are some useful tips:

  • 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_238
  • If this is your first PR, please add yourself to the AUTHORS.md file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-fix build Everything related to building NF
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants