Initialize members in CSCDQM_StripClusterFinder #40353
Conversation
- initialized member data - removed unused member data - moved member data to be stack variables passed to the functions
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40353/33446
|
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages:
@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-caf3d3/29674/summary.html Comparison SummarySummary:
|
|
+1
|
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40353/33620
|
|
Pull request #40353 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please check and sign again. |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-caf3d3/29858/summary.html Comparison SummarySummary:
|
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| void StripClusterFinder::KillCluster(void) { | ||
| uint32_t StripClusterFinder::KillCluster(uint32_t ic1, uint32_t ic2, C1 const& c1, C2 const& c2) { |
There was a problem hiding this comment.
This respects the structure of the original code and updates icstart.
However, there is no need to have this method uint, nor to update icstart inside the loop, when it was only used to set the initial value. This is also notified by the SA, which signals a dead assignment here.
I don't want to be too picky: let merge this PR as such now (i.e. same flow as in the original code). However, I think that in a follow up PR once this is merged one could revert this method being void and avoid updating icstart after its usage as starting point for the loop.
|
+1 |
PR description:
PR validation:
Code compiles.