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
Updating FastjetJetProducer and related producers with (factorized) fillDescriptions #19098
Updating FastjetJetProducer and related producers with (factorized) fillDescriptions #19098
Conversation
This reverts commit 57a4259.
A new Pull Request was created by @perrotta for master. It involves the following packages: HLTrigger/Configuration @perrotta, @cmsbuild, @silviodonato, @fwyzard, @Martin-Grunewald, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison is ready Comparison Summary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
looks good aside from the incorrect @ module_label in fillDescriptions. Please remove all of them from this package. |
-1 |
@davidlange6 : I think I removed @ module_label in fillDescriptions already with my last commit, while letting it as argument of getParameter upon request from Andrea B. Did I forget any? Could you please point it to me? I can't grep it... |
} | ||
VirtualJetProducer::VirtualJetProducer(const edm::ParameterSet& iConfig) { | ||
|
||
moduleLabel_ = iConfig.getParameter<string> ("@module_label"); |
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 @perrotta - here.
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.
This is intended.
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.
ah -grr - i'm lost. i mixed get and define
+1 |
hi @perrotta - it looks like this breaks workflows 301/302 (heavy ions) - because of the nSigma config changes. Could you have a look (i presume the fix is to remove a few lines from a cfi) |
Thak you @davidlange6 |
@davidlange6 : #19163 should fix the issue |
sorry I just saw this, thanks @perrotta |
Same as #18975 (later #19090), but here the different parts of the fillDescription methods are factorized out
@alefisico @rappoccio @ferencek @ahinzmann @nhanvtran @jdolen @gkasieczka
@fwyzard @Martin-Grunewald @silviodonato