-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add MSVC testing #347
Add MSVC testing #347
Conversation
I've pushed changes to the testsuite. CI is now happy |
One still need to properly fix conditional steps in the CI |
No, I'm unhappy about the conditional step change in the ci |
@gasche, this is now ready. |
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.
The mix of "foo" -.- bar
and "foo." ^ bar
in the testsuite is a bit jarring (it took me a few dozen seconds to convince myself than no, they were strictly equivalent), but maybe we can tolerate this if you find it cumbersome to cleanup, and generally the patch is nice to read. I am okay with having the commit that adds .exe to the install script as part of this PR.
fixed |
Some work needed on the testsuite, but starting to build on the truly astounding work already done on Windows support here!
Should fix #323 as we're now testing msvc