CI for C++ (formatting check + running tests)#2901
Conversation
| # execute process doesn't fail if the process fails. | ||
| # `COMMAND_ERROR_IS_FATAL ANY` parameter fixes this but is only available in CMake 3.19 | ||
| # which isn't default on Ubuntu LTS as of writing, causing unnecessary friction. | ||
| execute_process(COMMAND cargo build --release -p re_types RESULT_VARIABLE ret) # Generates most of the C++ source files |
There was a problem hiding this comment.
Good question/observation. Entirely because I stumbled over this line and figured maybe this is more cache friendly then
--release so we can inherit from some of the artifacts that maturin has just built before
but maybe this doesn't make sense?
Arguably the cmake script should be configurable and probably build rerun_c with debug/release depending on what rerun_cpp is built!
There was a problem hiding this comment.
In this instance this is running independently from other jobs so building re_types in release should just slow things down I think.
Arguably the cmake script should be configurable and probably build rerun_c with debug/release depending on what rerun_cpp is built!
Yeah that would be nice, though always building rerun_c in release isn't too bad in and of itself I think.
Update: oh wait, I see that rerun_c depends on re_types through re_sdk! In which case building in release above does make sense but... why does rerun_c depends on re_types to begin with? All it needs is like Data{Cell/Row} and RecordingStream. Maybe we should have an option an re_sdk to discard all of the archetype stuff.
There was a problem hiding this comment.
huh, yeah we have some work there to do! Creating an issue on making rerun_c smaller.
I'll keep the cmake as is for the moment, shouldn't be too impactful anyways - imho the important thing is that it's not fixed to Debug instead (i.e. don't force users to go through a debug c library)
|
Hello, |
What
Docker image script was in a broken state, fixed it and updated the image everywhere.
Do not under any circumstances review commit per commit 😉 (srsly the history is messed up)
TODO:
Future todo:
Checklist