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
Clean up a few dead assignments from CalibCalorimetry/HcalAlgos and RecoMuon/TrackingTools #31781
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31781/19040
|
A new Pull Request was created by @perrotta for master. It involves the following packages: CalibCalorimetry/HcalAlgos @perrotta, @yuanchao, @slava77, @christopheralanwest, @tocheng, @cmsbuild, @tlampen, @jpata, @pohsun 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 job queued. |
Comparison is ready Comparison Summary:
|
+1
|
For 'CalibCalorimetry/HcalAlgos/src/HcalLogicalMapGenerator.cc' the modification is just commended out the dead assignment. Would it be better just remove the whole line? or 'mytype = 3' is still meaningful somewhere? |
It was done on purpose, since I don't know if such an assignment was left as such with the intention of possibly reusing it in further developments. Of course, if the responsible of the code agrees that there is no reason to let such a "promemoria" in the code I will remove that line as a whole. |
Not remembering all the details of the HcalLogicalMapGenerator
(history), I'd suggest to keep the commented line.
…On Fri, 16 Oct 2020, perrotta wrote:
For 'CalibCalorimetry/HcalAlgos/src/HcalLogicalMapGenerator.cc' the
modification is just commended out the dead assignment. Would it be better
just remove the whole line? or 'mytype = 3' is still meaningful somewhere?
It was done on purpose, since I don't know if such an assignment was left as such with
the intention of possibly reusing it in further developments. Of course, if the
responsible of the code agrees that there is no reason to let such a "promemoria" in the
code I will remove that line as a whole.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, orunsubscribe.[ABGHJWQ3NX6HSGWN3PHBZDTSLAESNA5CNFSM4SQHILYKYY3PNVWWK3TUL52HS4DFVREXG43VMVB
W63LNMVXHJKTDN5WW2ZLOORPWSZGOFJIJXVY.gif]
|
Kind reminder @cms-sw/alca-l2 |
urgent |
+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, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Following some finding of the static analyzer, a few dead assignments are removed from the mentioned packages
I also took the opportunitiy to get rid of one extra sqrt computation in
MuonErrorMatrix::Term(const AlgebraicSymMatrix55 &curv, int i, int j)
PR validation:
It builds correctly