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

chore: run clang-tidy (linter) on all files when on main or manually triggered #504

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Taepper
Copy link
Collaborator

@Taepper Taepper commented Jun 28, 2024

resolves #498

Summary

PR Checklist

  • All necessary documentation has been adapted or there is an issue to do so.
  • The implemented feature is covered by an appropriate test.

Comment on lines +14 to +23
waitForDependencyImage:
name: Wait for Docker Image
runs-on: ubuntu-latest
steps:
- name: Wait for Docker Image
uses: lewagon/[email protected]
with:
ref: ${{ github.ref }}
repo-token: ${{ secrets.GITHUB_TOKEN }}
check-name: Build linter dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this job?

on:
  workflow_run:
    workflows: [Build Linter Image]

should already do the waiting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True. I did not know you can still define branch as a condition. But this does not work for the diffed files unfortunately as I need the PR number in the action to get the list of changed files. And I can only get it in the PR trigger event

Comment on lines +36 to +55
- shell: bash
name: Configure and run clang-tidy on changed files
run: |
mv /src/build .
cmake -DBUILD_WITH_CLANG_TIDY=on -D CMAKE_BUILD_TYPE=Debug -B build
echo "Successfully configured cmake"
files=$(curl -s \
"https://api.github.com/repos/${{ github.repository }}/pulls/${PR_NUMBER}/files" \
| jq -r '.[].filename')
echo "Changed files of this PR:"
echo "$files"
IFS=$'\n'
for file in $files; do
echo "Check ending for file: $file"
if [[ $file == *.cpp ]]; then
echo "Now linting the file: $file"
echo "cmake --build build --target ${file%.cpp}.o"
cmake --build build --target ${file%.cpp}.o
fi
done
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we simply use the old approach when running the linter on all files (i.e. simply building the code)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, yes, that was supposed to be the case. I was still experimenting with the action triggers and waits! Sorry for not marking it as draft!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-tidy on all files
2 participants