CI improvements #368
CI improvements #368
Conversation
| cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DTESTING=ON -DCMAKE_TOOLCHAIN_FILE="$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake" | ||
| cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target all --parallel 2 | ||
| cmake -B build -DTESTING=ON | ||
| cmake --build build --target all |
There was a problem hiding this comment.
I would add the --parallel 2 back. Even though we may only have once vcpu, we want to keep the compiler as busy as possible. I have always seen recommendations of cpu.count + 1.
|
|
||
| - name: Unit Test (OpenSSL 3) | ||
| run: | | ||
| cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" --target test | ||
| cmake --build build --target test |
There was a problem hiding this comment.
add --parallel 2
I won't bore you with adding this repeatedly.
|
|
||
| - name: Build (Interop Harness) | ||
| run: | | ||
| cd cmd/interop | ||
| cmake -B build -DCMAKE_TOOLCHAIN_FILE="$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake" | ||
| cmake -B build | ||
| cmake --build build |
| brew install llvm pkg-config | ||
| ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format" | ||
| ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy" | ||
| - uses: lukka/get-cmake@latest |
There was a problem hiding this comment.
for a build environment, I would try and lock in a version.
- uses: lukka/get-cmake@latest
+ uses: lukka/[email protected]| @@ -10,7 +10,7 @@ | |||
| "doctest", | |||
| "nlohmann-json" | |||
| ], | |||
| "builtin-baseline": "5908d702d61cea1429b223a0b7a10ab86bad4c78", | |||
| "builtin-baseline": "962e5e39f8a25f42522f51fffc574e05a3efd26b", | |||
There was a problem hiding this comment.
If this passes CI, that is great!
There was a problem hiding this comment.
I am seeing some warnings from the running jobs that might need to be addressed. One of them is regarding caching.
https://github.com/cisco/mlspp/actions/runs/6132855786
|
Closing this in favor of #367. Some of the cleanup here may still be relevant, but we should get that PR tuned up and merged and then revisit. |
include-regexas well ascheck-path