-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
added esConsumes to modules in L1Trigger /L1ExtraFromDigis #30336
added esConsumes to modules in L1Trigger /L1ExtraFromDigis #30336
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30336/16339
|
A new Pull Request was created by @JamminJones for master. It involves the following packages: L1Trigger/L1ExtraFromDigis @cmsbuild, @rekovic, @benkrikler can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
This code change was made under my supervision. |
hold the EventSetup gets are done conditionally so we should set the tokens conditionally as well. |
Pull request has been put on hold by @Dr15Jones |
esHandles were only being used based on the module parameters. Now the associated tokens are only set based on the same parameters.
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30336/16357
|
Pull request #30336 was updated. @cmsbuild, @rekovic, @benkrikler can you please check and sign again. |
please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
unhold |
NOTE: The new code conditionally calls esConsumes based on the conditional getting of the data from the EventSetup. This was done to avoid prefetching of EventSetup data that will not be needed. Looking at the existing I suggest that the maintainers of this code modify the |
Comparison is ready Comparison Summary:
|
ping |
ping (it has been 2 weeks now without a review) |
+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 will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
added esConsumes to 1 module
PR validation:
the code compiles