Run2-alca110 Modify two plugins to conform standard #21748
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21748/2680 |
|
A new Pull Request was created by @bsunanda for master. It involves the following packages: Calibration/HcalCalibAlgos @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
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:
|
| @@ -466,6 +478,10 @@ void RecAnalyzerMinbias::analyzeHcal(const HBHERecHitCollection & HithbheMB, | |||
| if (itr1 != histHC_.end()) itr1->second->Fill(energyhit); | |||
| } | |||
| h_[hid.subdet()-1]->Fill(energyhit); | |||
| if(energyhit >2) { | |||
| // Created: Thu Mar 4 18:52:02 CST 2010 | ||
| // | ||
| // | ||
|
|
There was a problem hiding this comment.
Isn't this part required for the CMSSW reference manual (of course, filled with content...)?
| void bookHistograms(); | ||
| void clearTreeVectors(); | ||
|
|
||
| bool debug_; |
There was a problem hiding this comment.
please make this configuration parameter const bool
|
|
||
| debug = iConfig.getUntrackedParameter<bool> ("Debug", false); | ||
| tok_jets_ = consumes<reco::GenJetCollection>(iConfig.getParameter<edm::InputTag>("JetSource")); | ||
| tok_parts_ = consumes<reco::GenParticleCollection>(iConfig.getParameter<edm::InputTag>("ParticleSource")); |
There was a problem hiding this comment.
these could also become const members and their initialization could be moved to the initializer list
| // Original Author: Jim Hirschauer (adaptation of Seema Sharma's | ||
| // IsolatedTracksNew) | ||
| // Created: Thu Nov 6 15:30:40 CST 2008 | ||
| // |
| desc.setUnknown(); | ||
| descriptions.addDefault(desc); | ||
| } | ||
|
|
There was a problem hiding this comment.
what about a non-default parameter set description?
| CaloSubdetectorGeometry* gHB = (CaloSubdetectorGeometry*) | ||
| (geo->getSubdetectorGeometry(DetId::Hcal,HcalBarrel)); | ||
| CaloSubdetectorGeometry* gHE = (CaloSubdetectorGeometry*) | ||
| (geo->getSubdetectorGeometry(DetId::Hcal,HcalEndcap)); |
There was a problem hiding this comment.
Why are casts required here?
Why removing const?
Why using C-style casts?
There was a problem hiding this comment.
This is corrected in a separate PR
There was a problem hiding this comment.
I do not want to make this change to have conflict in PR's
| hRawEta->Fill(eta1); | ||
| hRawPhi->Fill(phi1); | ||
| h_RawPt ->Fill(pt1); | ||
| h_RawP ->Fill(p1 ); |
There was a problem hiding this comment.
minor comment, but since you are anyway making a cosmetic change here, you could also remove the remaining whitespace in (p1 )
| double tempgen_TH[nPBins_+1] = { 0.0, 1.0, 2.0, 3.0, 4.0, | ||
| 5.0, 6.0, 7.0, 8.0, 9.0, | ||
| 10.0, 12.0, 15.0, 20.0, 25.0, | ||
| 30.0, 40.0, 50.0, 60.0, 70.0, 80.0, 100}; |
There was a problem hiding this comment.
I think here and also above you could replace this with std::array and just use the nice interface for iterating trough it
| @@ -1080,7 +1358,7 @@ void IsolatedTracksCone::beginJob() { | |||
| t_ievt = new std::vector<unsigned int>(); | |||
| t_ilum = new std::vector<unsigned int>(); | |||
There was a problem hiding this comment.
not part of this PR, but these vectors are never deleted
There was a problem hiding this comment.
These are part of a branch of a Tree. I am not sure if these to be deleted.
cmsbuild
added
code-checks-pending
comparison-pending
tests-pending
and removed
code-checks-approved
comparison-available
tests-approved
labels
cmsbuild
added
code-checks-pending
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21748/2831 |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
+1 |
cmsbuild
added
alca-approved
fully-signed
and removed
alca-pending
pending-signatures
labels
|
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 |
IsolatedParticlesGeneratedJets and IsolatedTracksCone are modified to conform current standards