CHG: MReadOutAssembly class clean up#167
Conversation
fhagemann
left a comment
fhagemann
left a comment
There was a problem hiding this comment.
Looks good to me.
Mostly style guide changes but also:
- Removing
StripHitTOnlymethods - Changing
m_InDetectorfrombool[16]toarray<bool,16> - Adding a comment about a
BUG? --> If the bug still persists, keep track of it in an issue?
| // Remove a strip hit | ||
|
|
||
| if (i < m_StripHits.size()) { | ||
| // BUG: MHit objects retain non-owning references to this strip hit if the caller deletes it. |
There was a problem hiding this comment.
Is this bug still present or does it need to be fixed? If the latter: track as issue?
|
And the GitHub Action tests all pass :) |
|
I removes all the "." at the end of the one-line comments. |
|
New version ready. |
|
Looks like #155 added new code to |
|
I checked the new code, fixed the formatting, and found a bug in it. Turns out part of it was already there in the old code too... that one was above Claude's and GPT's pay grade. |
|
I remember commenting on this in #124 (comment), but we should have also added a bug comment and opened an issue already back then.. 😅 |
|
Good catch :) |
2 participants
Lot's of changes, but 99% are just code, documentation & formatting clean-up.
Unit tests should pass.