refactor MonitorObject and adding Copy capabilities #2450
Conversation
Barthelemy
left a comment
Barthelemy
left a comment
There was a problem hiding this comment.
Your hands must be dirty and dusty after touching such an antique piece of code !
I agree with all this.
Not sure if we should merge it now or wait post-HI.
| void releaseObject(); | ||
| void cloneAndSetObject(const MonitorObject&); | ||
|
|
||
| ClassDefOverride(MonitorObject, 12); |
There was a problem hiding this comment.
you must increase the class version otherwise ROOT can't do the schema evolution.
not super important now that we don't store MO in the QCDB but still...
|
the thing is that I need this PR for QC-1201. But I don't know how necessary that ticket is... probably not much for HI. So I am 100% fine to just wait after HI... |
justonedev1
force-pushed
the
refactor_MO
branch
from
d904a4d to
857e3f5
Compare
knopers8
left a comment
knopers8
left a comment
There was a problem hiding this comment.
As I was talking to you the other day, I am not very happy about having a unique_ptr which does not always own the pointer, but I don't see a better way to do this. And I support the intention of the changes. Thank you for the initiative!
justonedev1
force-pushed
the
refactor_MO
branch
from
857e3f5 to
c83da24
Compare
|
@ktf Can you please take a look at the reason why the build checks are failing? |
3 participants
when working on QC-1201 and implementing Copy function/ctor/operator I found that Monitor Object is weirdly written, moreover there are possiblities, where the class can leak memory. So I refactored it a bit while adding Copy function, ctor and operator.