Various fixes and updates for CI #2497
Conversation
ZedThree
commented
Feb 15, 2022
ZedThree
commented
- Have a build with as few dependencies as possible
- Remove the OpenMP build -- covered by other builds
- Automatically apply clang-format to PRs
- Only run certain workflows on changes to certain files
- black on changes to Python files
- clang-format and clang-tidy on changes to C++ files
- Bump clang-tidy-review version
- Now makes suggestions from fixits that can be more easily applied
- Plus cleaner config
- Cancel previous workflows on new pushes to a branch/PR
CI is skipped automatically, and the tag causes CI to then be skipped on merge commits that include the commit message
Already covered by other builds
Probably a bug?
| on: | ||
| push: | ||
| paths: | ||
| - '**.py' |
There was a problem hiding this comment.
Note we also ship executables without py extension ...
There was a problem hiding this comment.
Fair point, probably bin/** is sufficient here?
There was a problem hiding this comment.
Unfortunately not:
$ git grep '#!/.*/bin/.*python'|cut -d: -f1|grep -v \\\.py\$|grep -v runtest
bin/bout-add-mod-path
bin/bout-boutdata-restart-create
bin/bout-pylib-cmd-to-bin
bin/bout-pylib-cmd-to-bin
bin/bout-sonnet
examples/conduction/run
examples/elm-pb/runexample
examples/laplace-petsc3d/run
examples/staggered_grid/runandplot
manual/sphinx/user_docs/python_boutcore.rst
tests/integrated/test_suite
tests/requirements/metric_3d
tests/requirements/scipy
tools/tokamak_grids/elite/elite2nc
tools/tokamak_grids/gato/gato2nc
| -DBOUT_BUILD_EXAMPLES=ON \ | ||
| -DBOUT_BUILD_DOCS=OFF \ | ||
| -DBOUT_ENABLE_PYTHON=OFF \ | ||
| -DCMAKE_EXPORT_COMPILE_COMMANDS=On |
There was a problem hiding this comment.
| -DCMAKE_EXPORT_COMPILE_COMMANDS=On | |
| -DCMAKE_EXPORT_COMPILE_COMMANDS=ON |
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Does it make sense to run the tests twice for push+PR?
Or is that already sufficient to prevent this?
There was a problem hiding this comment.
This particular workflow only runs on PRs, but the main tests yes, run on both push and PR to check the merge is ok.
There was a problem hiding this comment.
Is there a way to check whether that changes anything? For most PRs they are based on the base branch, this is doing the same runs twice - if we could skip that, that would safe a lot of CPU time ...
But unless it is easy (which I worry it isn't) probably not worth to pursue ...
There was a problem hiding this comment.
That's a really good question! I guess if there are commits to the base branch that are more recent than any commits in the current branch, then it's worth doing the PR jobs? It's probably worth looking to see if there's something already existing that could handle that for us
There was a problem hiding this comment.
It looks to me like this is what we want:
https://github.com/fkirc/skip-duplicate-actions#example-1-skip-entire-jobs
Ensure all files get formatted
