Add DeveloperStudio CI coverage and fix USE_TLS=1 builds#270
Add DeveloperStudio CI coverage and fix USE_TLS=1 builds#270michael-grunder merged 2 commits intomainfrom
Conversation
* We need to use the DeveloperStudio specific `-mt` flag * Fix a makefile race when building our tests Signed-off-by: michael-grunder <michael.grunder@gmail.com>
bjosv
left a comment
There was a problem hiding this comment.
Nice, I have some comments but it's your call.
.github/workflows/build.yml
Outdated
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Clean project | ||
| run: git clean -xfd |
There was a problem hiding this comment.
Is the clean step needed for these Solaris builds?
I don't think we do any cleaning in other jobs, and I hoped that the previous checkout step would be a fresh checkout.
There was a problem hiding this comment.
I think it was because each matrix build (USE_TLS=0, USE_TLS=1) still had the intermediate build artifacts from the prior build, but it's not neccessary if we just do one build.
.github/workflows/build.yml
Outdated
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| use_threads: [0, 1] |
There was a problem hiding this comment.
I'm not sure we need to test all variants on Solaris, we can probably use less CPU-time on Github if we just build with USE_TLS=1 and USE_THREADS=1.
The USE_TLS=1 would probably cover everything in USE_TLS=0 at least.
Signed-off-by: michael-grunder <michael.grunder@gmail.com>
d36f904 to
7a801ca
Compare
|
Yeah i think that's better. No reason to permutate each build option. |
USE_TLS=1on that compilerI've never opened a PR requiring secrets in the upstream repo and apparently you can't do it from your fork.