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
70 x consumes migration i #1106
Conversation
A new Pull Request was created by @vadler (Volker Adler) for CMSSW_7_0_X. 70 x consumes migration i It involves the following packages: DQM/SiStripMonitorCluster @smuzaffar, @thspeer, @danduggan, @rovere, @nclopezo, @deguio, @slava77, @vadler, @eliasron can you please review it and eventually sign? Thanks. |
Hi, Now there are conflicts in the files: DQMOffline/Muon/interface/MuonRecoOneHLT.h with respect to the current HEAD of the branch. |
Ah, lots of new stuff in the latest IB since pre6, which was the most recent one available for development last night :( |
@@ -48,12 +49,12 @@ | |||
#include "GlobalVariables.h" | |||
|
|||
|
|||
class CaloMETAnalyzer : public CaloMETAnalyzerBase { | |||
class CaloMETAnalyzer : public CaloMETAnalyzerBase, public edm::EDConsumerBase { |
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'm afraid inheriting from EDConsumerBase won't work since the framework only asks modules what they consume. Filling 'consumes' here will still cause the framework to abort any module which uses this code.
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.
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.
'cause sometimes people think smarter.
I fear some restructuring of the code is needed here. I'll discuss with @deguio @danduggan and see how to proceed.
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.
BTW: The change causing the conflict in MuonRecoOneHLT (#1106 (comment)) resolved that issue at least there. It was ported to be an EDAnalyzer.
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.
It looks like I misunderstood the code structure of that package.
Fix is here: vadler@b3fc2a2
Pull request #1106 was updated. @smuzaffar, @thspeer, @danduggan, @rovere, @nclopezo, @deguio, @slava77, @vadler, @eliasron can you please check and sign again. |
Pull request #1106 was updated. @smuzaffar, @thspeer, @danduggan, @rovere, @nclopezo, @deguio, @slava77, @vadler, @eliasron can you please check and sign again. |
Pull request #1106 was updated. @smuzaffar, @thspeer, @danduggan, @rovere, @nclopezo, @deguio, @slava77, @vadler, @eliasron can you please check and sign again. |
+1 |
+1 |
This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it? |
Consumes migration -- CommonTools/TriggerUtils
Migration of CommonTools/TriggerUtils to 'consumes'/'getByToken'-API.