-
Notifications
You must be signed in to change notification settings - Fork 3
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
base: main
Are you sure you want to change the base?
Conversation
permissions: packages: read
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
- 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 |
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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!
resolves #498
Summary
PR Checklist