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
fix of TrackSplitting module #11792
fix of TrackSplitting module #11792
Conversation
A new Pull Request was created by @fioriNTU for CMSSW_7_4_X. fix of TrackSplitting module It involves the following packages: DQM/TrackingMonitor @cmsbuild, @danduggan, @vanbesien, @deguio can you please review it and eventually sign? Thanks. |
@@ -148,15 +156,6 @@ void TrackSplittingMonitor::bookHistograms(DQMStore::IBooker & ibooker, | |||
void TrackSplittingMonitor::beginJob(void) |
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 suggest to remove beginJob and ednJob methods completely: they are bogus for this module type (framework will never call them, either single thread or multithread). Also, add override
in the header file to the virtual derived methods.
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.
Ok, thanks for suggestions!
Il 14/10/2015 17.15, Slava Krutelyov ha scritto:
In DQM/TrackingMonitor/src/TrackSplittingMonitor.cc
#11792 (comment):@@ -148,15 +156,6 @@ void TrackSplittingMonitor::bookHistograms(DQMStore::IBooker & ibooker,
void TrackSplittingMonitor::beginJob(void)I suggest to remove beginJob and ednJob methods completely: they are
bogus for this module type (framework will never call them, either
single thread or multithread). Also, add |override| in the header file
to the virtual derived methods.—
Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/11792/files#r42007051.
Questa e-mail è stata controllata per individuare virus con Avast antivirus.
https://www.avast.com/antivirus
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.
Hi Slava,
can you confirm that the "override" is to be added only in the analyze
method in the header?
FF
Il 14/10/2015 17.15, Slava Krutelyov ha scritto:
In DQM/TrackingMonitor/src/TrackSplittingMonitor.cc
#11792 (comment):@@ -148,15 +156,6 @@ void TrackSplittingMonitor::bookHistograms(DQMStore::IBooker & ibooker,
void TrackSplittingMonitor::beginJob(void)I suggest to remove beginJob and ednJob methods completely: they are
bogus for this module type (framework will never call them, either
single thread or multithread). Also, add |override| in the header file
to the virtual derived methods.—
Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/11792/files#r42007051.
Questa e-mail è stata controllata per individuare virus con Avast antivirus.
https://www.avast.com/antivirus
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.
yes
... if you add it to beginJob, the compiler will tell you that it's not a method inherited from the base class, which is a pretty good indicator that the method is not right (and in this case should be removed)
Pull request #11792 was updated. @cmsbuild, @danduggan, @vanbesien, @deguio can you please check and sign again. |
please test |
+1 |
The tests are being triggered in jenkins. |
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs after it passes the integration tests and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
fix of TrackSplitting module
Make this module usable in multi thread