-
Notifications
You must be signed in to change notification settings - Fork 15
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
Setup CI with Github Actions #1
Conversation
0dd17cb
to
78be53d
Compare
Failing fast means we won't know if all platforms are affected or whether it's just a single platform that's problematic. So disable this, we're failing fast enough anyway.
312bde3
to
dd2bb5c
Compare
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.
Very impressive work! Not much to comment, some comments are just questions so I better understand the why.
I wonder if we shouldn't make fmt the default and embrace it once and for all. We can keep the code to use std::format around.
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.
Looks good, let's merge and build on top of that. Thank you!
For now, drop support for std::format. It's not supported by the compilers used in our CI on ubuntu & macOS At least on my local machine, this means we need to disable -Werror, as one of the templates in the bundled format library produces a -Werror=dangling-reference. We should try to reenable this when possible.
This hopefully fixes macOS CI
In CI, we want to be as close to real deployment as possible, which would be release/release with debug information.
This should allow us to reenable -Werror. It might even fix the SegFault we've experienced in CI on Windows 🤞
Test whether this finally solves our Windows CI issues
4b67296
to
47b47f5
Compare
Change-Id: I2d94de3c722afdf25b1a9d20b1288dece9d94969
Just use the fmt lib bundled with it. Change-Id: I83d9974ebe63e74dee947f7b7d6cabfb959c6c83
Change-Id: I413f05fa409294978b470afd393eb6fa02867fba
b416813
to
752d137
Compare
Also when running the tst_knut. Change-Id: I56f7fa34d45cd2236511427546130dd1d006717e
Unfortunately I cannot formally approve, as I'm the author of the PR, but this PR is good enough for now. |
First set up to build & test knut using Github Actions instead of our own CI.
This is meant to be a first working version, not a complete CI setup.
Remaining ToDos after merging this PR can be found in #3