Remove a dead assignment in CondFormats/SiStripObjects/src/SiStripThreshold.cc #46174
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46174/41972 |
|
A new Pull Request was created by @perrotta for master. It involves the following packages:
@atpathak, @cmsbuild, @consuegs, @francescobrivio, @perrotta can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
| : 512 - | ||
| firstStrip; //*FIXME, I dont' know ithis class the strip number of a detector, so I assume wrongly that if the last firstStrip<511 the detector has only 512 strips. Clearly wrong. to be fixed | ||
|
|
||
| //*FIXME, I dont' know ithis class the strip number of a detector, so I assume wrongly that if the last firstStrip<511 the detector has only 512 strips. Clearly wrong. to be fixed |
There was a problem hiding this comment.
since you are touching it can you fix this sentence? there's a typo and it doesn't make sense in English.
There was a problem hiding this comment.
Rewritten in (hopefully) some better English
| firstStrip; //*FIXME, I dont' know ithis class the strip number of a detector, so I assume wrongly that if the last firstStrip<511 the detector has only 512 strips. Clearly wrong. to be fixed | ||
|
|
||
| //*FIXME, I dont' know ithis class the strip number of a detector, so I assume wrongly that if the last firstStrip<511 the detector has only 512 strips. Clearly wrong. to be fixed | ||
| stripRange = firstStrip > 511 ? 768 - firstStrip : 512 - firstStrip; |
There was a problem hiding this comment.
Just for the record, I think one could make this exact by means of using the StripTopology (you can get it from this method
)
the con is that one would need to pass a
TrackerGeometry in input to SiStripThreshold::printSummary.An alternative way is to use the mechanism from
and use the fact that
unsigned int nStrip = detInfo_.getNumberOfApvsAndStripLength(detid).first * sistrip::STRIPS_PER_APV;
perrotta
force-pushed
the
removeDeadAssignmentSiStripThreshold
branch
from
3667f0e
to
f64ca2d
Compare
cmsbuild
added
tests-pending
code-checks-pending
and removed
tests-started
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46174/41977 |
|
please test |
|
+1 Size: This PR adds an extra 20KB to repository Comparison SummarySummary:
|
|
+1 |
cmsbuild
added
alca-approved
fully-signed
db-approved
and removed
alca-pending
db-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. @rappoccio, @mandrenguyen, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Trivial PR only intended to silence a warning issued by the static analyzer.
I also profit to reformat in a more readable way a comment line in it.
PR validation:
It compiles.
Nothing relevant is modified, nothing should change in outputs.