-
Notifications
You must be signed in to change notification settings - Fork 176
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
chore: readability-redundant-smartptr-get #4064
base: main
Are you sure you want to change the base?
chore: readability-redundant-smartptr-get #4064
Conversation
Warning Rate limit exceeded@paulgessinger has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughChanged, this pull request has. The clang-tidy configuration now be in a multi-line format with a new check— Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
841a44d
to
b25e8f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp (1)
71-77
: Wise changes, these are! Simplified smart pointer dereferencing, you have achieved.Proper null check before dereferencing, maintained you have. Direct dereferencing of
unique_ptr
, cleaner it is, than using.get()
. The Force is strong with this code, hmm.Remember, young Padawan: When dealing with smart pointers throughout the codebase, direct dereferencing prefer you should. The path to cleaner code, this is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.clang-tidy
(1 hunks)CI/clang_tidy/limits.yml
(1 hunks)Core/include/Acts/Geometry/BoundarySurfaceT.hpp
(1 hunks)Core/src/Detector/DetectorVolume.cpp
(2 hunks)Core/src/Detector/Portal.cpp
(1 hunks)Core/src/Material/MaterialMapper.cpp
(2 hunks)Core/src/Surfaces/ConeSurface.cpp
(1 hunks)Core/src/Surfaces/CylinderSurface.cpp
(1 hunks)Core/src/Surfaces/DiscSurface.cpp
(1 hunks)Core/src/Surfaces/LineSurface.cpp
(1 hunks)Core/src/Surfaces/PlaneSurface.cpp
(1 hunks)Examples/Algorithms/Geant4/src/Geant4Simulation.cpp
(1 hunks)Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp
(1 hunks)Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp
(1 hunks)Examples/Io/Json/src/JsonSurfacesWriter.cpp
(1 hunks)Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp
(1 hunks)Plugins/Geant4/src/Geant4Converters.cpp
(4 hunks)Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelBlueprintCreater.hpp
(1 hunks)Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp
(1 hunks)Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp
(1 hunks)Plugins/Json/src/SurfaceJsonConverter.cpp
(1 hunks)Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp
(1 hunks)Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp
(1 hunks)Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp
(1 hunks)Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- Plugins/DD4hep/src/DD4hepDetectorSurfaceFactory.cpp
- Plugins/GeoModel/include/Acts/Plugins/GeoModel/GeoModelDetectorElement.hpp
🚧 Files skipped from review as they are similar to previous changes (15)
- Core/src/Surfaces/DiscSurface.cpp
- Core/src/Surfaces/PlaneSurface.cpp
- CI/clang_tidy/limits.yml
- Core/src/Surfaces/CylinderSurface.cpp
- Tests/UnitTests/Core/Geometry/ProtoLayerTests.cpp
- Tests/UnitTests/Plugins/TGeo/TGeoParserTests.cpp
- .clang-tidy
- Core/src/Detector/Portal.cpp
- Core/src/Surfaces/LineSurface.cpp
- Tests/UnitTests/Core/TrackFitting/FitterTestsCommon.hpp
- Core/src/Material/MaterialMapper.cpp
- Core/src/Surfaces/ConeSurface.cpp
- Core/include/Acts/Geometry/BoundarySurfaceT.hpp
- Tests/UnitTests/Plugins/Geant4/Geant4DetectorElementTests.cpp
- Core/src/Detector/DetectorVolume.cpp
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: merge-sentinel
- GitHub Check: unused_files
- GitHub Check: missing_includes
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: macos
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: build_debug
- GitHub Check: linux_ubuntu
🔇 Additional comments (7)
Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp (1)
354-355
: Approve this change, I do!Cleaner and more idiomatic, the direct dereferencing with
*confined
is, than using.get()
. Improved readability without changing behavior, this does. Hmmmm.Plugins/GeoModel/src/GeoModelDetectorElementITk.cpp (1)
103-103
: Approve this change, I do!Simplified the smart pointer dereferencing has been, hmm. Direct dereferencing of
itkEl
, cleaner it is than using redundant.get()
. The Force flows better through this code now, it does.Examples/Io/Json/src/JsonSurfacesWriter.cpp (1)
112-113
: Approve this change, I do! Hmmmm.Improved readability, this change has brought. Direct dereferencing of smart pointer
confined
, cleaner and more idiomatic it is, than using.get()
. Safe this transformation remains, as ownership semantics unchanged they are.Examples/Algorithms/Geant4/src/Geant4Simulation.cpp (1)
95-97
: Approve this change, I do!Simplified the smart pointer dereferencing is. Redundant
.get()
call removed, yet same functionality maintained. Cleaner and more readable, the code now is, hmm.Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp (1)
29-29
: Wise change, this is! Removed redundant.get()
, you have.Cleaner and more idiomatic C++, the direct dereferencing is. Protected by null check above, it remains. Hmmmm.
Plugins/Json/src/SurfaceJsonConverter.cpp (1)
47-47
: Improved smart pointer usage, hmm, yes!Direct dereferencing of shared pointers, more elegant it is. Unnecessary
.get()
calls, removed they were.Also applies to: 53-53
Plugins/Geant4/src/Geant4Converters.cpp (1)
372-372
: Consistent improvement in smart pointer usage, I sense!Direct dereferencing of surface pointer in assignMaterial calls, cleaner code it creates. Removed
.get()
calls, unnecessary they were.Also applies to: 389-389, 406-406, 440-440
Quality Gate passedIssues Measures |
First without the necessary code changes so I can see if #4062 (which this is based on) works correctly.
--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat
,fix
,refactor
,docs
,chore
andbuild
types.Summary by CodeRabbit