-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[HeaderCheck][RECO3] Cleanup headers to avoid header inconsistency errors #26662
Conversation
smuzaffar
commented
May 6, 2019
- Remove obsolete header
- Deleted header which are not in use
The code-checks are being triggered in jenkins. |
please test |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26662/9623
|
The tests are being triggered in jenkins. |
A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master. It involves the following packages: RecoEcal/EgammaClusterAlgos @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Comparison job queued. |
|
||
|
||
/// A service to retrieve to provide a hook to EcalSeverityLevelAlgo | ||
class EcalSeverityLevelService { |
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.
@argiro
please check and confirm that deletion is OK.
Thank you.
#include <utility> | ||
|
||
template <typename T, typename C> | ||
class ElementsInEllipse{ |
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.
@mbluj @steggema @swozniewski
please check and confirm that this is not needed anymore.
It looks like this was (re)added in 2013 for HLT compatibility
0f45b61#diff-b027deb3da39d07e90367d5113d72d58
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 indeed looks as pretty old, unused code - @veelken do you remember what the code was used for (for TaNC?) and if it can be safely removed?
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 can confirm that this is on the list of files that we're considering for removal in an upcoming cleanup, and also that it is not used by current Tau HLT algorithms (and I don't think we intend to run Run-1 tau HLT algorithms with future CMSSW).
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 will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |