WIP: 22.0-cleanup-SiClusterizationTool
Primarily improve error reporting so use of at()
with an index out of range is trapped and reports a meaningful ATH_MSG_FATAL message instead of creating an exception (in context of ATLIDTRKCP-302). Other cleanups done: use of cmath constants instead of TMath, tidy up includes, use of std::array instead of a c style array, clarification of the declaration and usage of a pointer-to-member-function variable.
Change to WIP while urgent changes go in (!38018 (merged))
Merge request reports
Activity
added InnerDetector master review-pending-level-1 labels
✅ CI Result SUCCESS (hash ae80f90a)Athena AthSimulation AthGeneration AnalysisBase externals ✅ ✅ ✅ ✅ cmake ✅ ✅ ✅ ✅ make ✅ ✅ ✅ ✅ required tests ✅ ✅ ✅ ✅ optional tests ✅ ✅ ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
✅ AthGeneration: number of compilation errors 0, warnings 0
✅ AnalysisBase: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 23079]- Resolved by Shaun Roe
- Resolved by Shaun Roe
Thanks @sroe, this is a really nice clean up. In addition to my one question above I had a couple of more general comments
- There are places where the indentation looks to have gone a bit off (particularly around log messages). This may well be deliberate but if not I thought I'd let you know.
- I don't think the ATLAS c++ style guide says anything specifically about whether to use the word or symbol form of logical operators, but I think it would be good to be consistent within a single file (
and
vs&&
,not
vs!
,or
vs||
)
Tom - L1 shifter
Edited by Thomas James Neep
mentioned in merge request !38018 (merged)
✅ CI Result SUCCESS (hash 0490699a)Athena AthSimulation AthGeneration AnalysisBase externals ✅ ✅ ✅ ✅ cmake ✅ ✅ ✅ ✅ make ✅ ✅ ✅ ✅ required tests ✅ ✅ ✅ ✅ optional tests ✅ ✅ ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
✅ AthGeneration: number of compilation errors 0, warnings 0
✅ AnalysisBase: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 23157]added review-approved label and removed review-pending-level-1 label
@sroe , sorry for not merging this one earlier. Yesterday I had to put a few merge requests on hold because we were preparing good nightly for 22.0.20.
Meanwhile we got merge conflicts on this one. Could you please fix them? Thanks.
added review-user-action-required label
Hi @tsulaia , in fact I intended this to be delayed once I saw that Gabriel F. and Dan G. had urgent fixes to go in. I will see what I can do once all their MRs are in; possibly their changes have made this one redundant.
mentioned in merge request !38231 (merged)