Update macOS build and CI maintenance. #351
Conversation
samcunliffe
added
technical
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
| lcov --remove coverage.info '/Library/*' --output-file coverage.info | ||
| lcov --remove coverage.info '/Applications/*' --output-file coverage.info | ||
| lcov --ignore-errors unused --remove coverage.info '/Library/*' --output-file coverage.info | ||
| lcov --ignore-errors unused --remove coverage.info '/Applications/*' --output-file coverage.info |
There was a problem hiding this comment.
It was erroring because /Library/* wasn't found. Since we restrict this to ubuntu builds, we could also straight remove lines 181 and 182, but it feels like it's worth maintaining a macOS capability here.
| on: | ||
| push: | ||
| branches: ["main"] | ||
| pull_request: | ||
| merge_group: | ||
| schedule: | ||
| - cron: "0 9 15 * *" |
There was a problem hiding this comment.
Since the build is brittle, I propose we run it monthly to keep vaguely on top of things...?
samcunliffe
changed the title
upload-artifact, download-artifact and checkout versions.
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes CI and tooling by updating action versions, improving build configurations (especially on macOS/ARM), and adding linting steps.
- Bump
checkout,upload/download-artifact, andsetup-matlabactions across workflows - Clean up CMake for ARM macOS, fix
libompand compiler warnings, and add monthly CI schedule - Introduce clang-format checks, update pre-commit hooks, and tidy interpolation test formatting
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tdms/tests/unit/test_interpolation_functions.cpp | Reformatted empty while loop in interpolation test |
| tdms/tests/system/utils.py | Added blank line after module docstring |
| tdms/src/simulation_parameters.cpp | Qualified std::move for string parameters |
| tdms/src/simulation_manager/execute_simulation.cpp | Reformatted OpenMP #pragma argument list |
| tdms/CMakeLists.txt | Removed forced x86_64 architecture for macOS |
| .pre-commit-config.yaml | Updated hook revisions for isort, black, autoflake, pre-commit-hooks |
| .github/workflows/matlab_tests.yml | Bumped matlab-actions/setup-matlab to v2.5.0 |
| .github/workflows/doxygen-gh-pages.yml | Upgraded actions/checkout to v4 and artifact actions to v4 |
| .github/workflows/ci.yml | Added monthly cron, clang-format validation, updated actions to v4, tweaked matrix and macOS deps |
| int n = 8; | ||
| while (--n > 0 && abs(coeff_sums[n] - coeff_sums[0]) < tol) | ||
| ; | ||
| while (--n > 0 && abs(coeff_sums[n] - coeff_sums[0]) < tol); | ||
| // we only reach the end of the while loop, IE get to n==0, when all elements | ||
| // in the array are the same | ||
| REQUIRE(n == 0); |
There was a problem hiding this comment.
[nitpick] The empty loop body with only a trailing semicolon can be hard to read; consider using std::all_of to express the intent more clearly or add an inline comment to highlight that the loop is intentionally empty.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I'm not going to do that here, but it looks like a good suggestion!
|
@willGraham01, sorry for the noise. This PR snowballed. But it's green now. 😌 🟢 |
Merge commits are not allowed on this repository
What started as a simple update to the deprecated versions GitHub's
upload-artifact,download-artifactandcheckoutactions has snowballed into major housekeeping.upload-artifact,download-artifactandcheckout;setup-matlabsince they introduced asudobug on Windows and macOS;clang-formatlinter suggestions;libomperror and some compiler warnings on the macOS build;We should be sure to squash-merge this mess!