-
Notifications
You must be signed in to change notification settings - Fork 147
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
[WIP] [QC-1176] Display a warning for developers if a registered object is not mergeable #2400
base: master
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -60,7 +60,7 @@ ObjectsManager::~ObjectsManager() | |||||
ILOG(Debug, Devel) << "ObjectsManager destructor" << ENDM; | ||||||
} | ||||||
|
||||||
void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicationPolicy) | ||||||
void ObjectsManager::startPublishingImpl(TObject* object, PublicationPolicy publicationPolicy) | ||||||
{ | ||||||
if (!object) { | ||||||
ILOG(Warning, Support) << "A nullptr provided to ObjectManager::startPublishing" << ENDM; | ||||||
|
@@ -70,6 +70,9 @@ void ObjectsManager::startPublishing(TObject* object, PublicationPolicy publicat | |||||
ILOG(Warning, Support) << "Object is already being published (" << object->GetName() << "), will remove it and add the new one" << ENDM; | ||||||
stopPublishing(object->GetName()); | ||||||
} | ||||||
if (mergers::isMergeable(object)) { | ||||||
ILOG(Warning, Support) << "Object '" + std::string(object->GetName()) + "' with type '" + std::string(object->ClassName()) + "' is not one of the mergeable types, it might cause issues during publishing"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
missing ENDM and rephrasing |
||||||
} | ||||||
auto* newObject = new MonitorObject(object, mTaskName, mTaskClass, mDetectorName); | ||||||
newObject->setIsOwner(false); | ||||||
newObject->setActivity(mActivity); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -257,7 +257,7 @@ void TrendingTask::generatePlots() | |
} | ||
auto c = drawPlot(plotConfig); | ||
mPlots[plotConfig.name].reset(c); | ||
getObjectsManager()->startPublishing(c, PublicationPolicy::Once); | ||
getObjectsManager()->startPublishing<true>(c, PublicationPolicy::Once); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should not need |
||
} | ||
} | ||
|
||
|
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.
I think this should be also disabled by the presence of
QUALITYCONTROL_POSTPROCESSINTERFACE_H
andIgnoreMergable==true
, otherwise we will see a lot of spurious warnings.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.
@justonedev1 can you explain how your most recent changes make
QUALITYCONTROL_POSTPROCESSINTERFACE_H
disable this warning? I'm not convinced they do.