The VTK dashboards are scary. I know a lot is going on, but the current state worries me. Should we go into rapid response mode and fix this ? I see several sloppy warnings/errors that have been around for a while, at a minimum we can clean that junk up.
The current test failures are from this MR and is being reverted in this MR. The build failures from out of space…I’m on that now; it seems one of the CI machines has just a 200G disk which shouldn’t be a thing (we aim for 1TB at least).
The transition to CI is currently disruptive, but I’ve been trying to keep
master clean. Developers not running CI or assuming problems aren’t theirs seem to be the main issue right now.
Thanks, I will address some slop issues today
This MR fixes a warning in the Imprint filter if you’re looking at that one.
Thanks! Ben I notice builds like https://open.cdash.org/build/7182539 are recommending replacing vector<>::push_back with emplace_back. (I’ve been doing this for a while now anyway in new classes.) Do you think it’s a good idea to do this wherever push_back is found?
No, it shouldn’t be a knee-jerk reaction. It should be done when a move is possible and better. For example:
std::vector<Type> v; // always better as v.emplace_back(/*ctor args*/); v.push_back(Type(/*ctor args*/)); // Only better if `t` isn't used after the insertion. Type t; v.push_back(t); // Always better. v.push_back(std::move(t)); // `t` is "dead" after this anyways
For more details, this blog post might help.
Some weeks ago, I had my bots (the Rogue Research ones) down to zero errors/warnings but just days later there were new ones. I have this suspicion that no one looks at cdash anymore and only looks at gitlab.
We can add warning flags to the CI bots to match the Rogue machines. The thing is that waiting for CI on MRs, then also checking CDash once it has been merged for followups (without a way to verify warning fixes) is not any easier.
Right now, there are a bunch of VTK-m deprecation warnings that fire on the submodule I just updated to, but it’s a similar thing as other cases: things just need to move forward or I’m stalled out waiting for feedback on what to do which isn’t viable because everybody else is busy too.
VTK-m warnings (from non-Fides code) should be resolved in https://gitlab.kitware.com/vtk/vtk/-/merge_requests/7879
A fix for the DICOMParser warning is !7882.