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
Removed all EgammaAnalysis packages except for ElectronTools #26488
Removed all EgammaAnalysis packages except for ElectronTools #26488
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26488/9343
|
A new Pull Request was created by @guitargeek (Jonas Rembser) for master. It involves the following packages: EgammaAnalysis/CSA07Skims @cmsbuild, @smuzaffar, @Dr15Jones, @santocch can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
I'm totally fine with this :D Thanks Jonas :) |
"EgammaAnalysis/Configuration", "EgammaAnalysis/ElectronIDESSources", | ||
"EgammaAnalysis/PhotonIDProducers", "ElectroWeakAnalysis/Configuration", | ||
"DiffractiveForwardAnalysis/Skimming", | ||
"ElectroWeakAnalysis/Configuration", |
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 seem to recall that this file is supposed to contain all known packages from the past, not just the current release.
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.
Really? I did the same thing in another PR where I removed a package:
#26186
Back then, @Dr15Jones was giving a +1 for that. So maybe I should also revert that change?
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.
nevermind, I may have misunderstood/confused with another file with a list of packages.
This one is in the core signautre list anyways and will be confirmed.
@Sam-Harper |
Indeed it is fine by me. If anybody is actually using it, we can always recover it from git. However I very much doubt it. In general, there is going to be a lot more of this happening for 11_X, I want to remove as much code as possible, erring on the side of removing when in doubt as we can always add it back in later. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
+1 |
+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 |
PR description:
As I have seen on the egamma meeting at the last CMS week, one of the plans is to get rid of some old ID code. Why not start here with these old skims and IDs? Are there used for anything @Sam-Harper?
PR validation:
Code compiles and matrix tests pass.