-
Notifications
You must be signed in to change notification settings - Fork 97
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
DO-NOT-MERGE: debug #1514 #1515
base: dev
Are you sure you want to change the base?
DO-NOT-MERGE: debug #1514 #1515
Conversation
WalkthroughWalkthroughThe changes encompass a variety of modifications across several files. Key adjustments include enhancing log output in a test script, refining the destructor in a transport device class, improving header inclusions and type assertions, initializing class members, adding new methods for listing objects, and updating build configurations. Additionally, a new method was introduced to manage a non-owning Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant TestScript
participant LogFile
participant FairMQTransportDevice
participant FairModule
participant FairGeoInterface
participant FairGeoLoader
participant FairGeoSet
User->>TestScript: Run test-splitMQ.sh.in
TestScript->>LogFile: Display 100 lines for failed devices
FairMQTransportDevice->>FairMQTransportDevice: Destructor with logging and conditional method call
FairModule->>FairModule: Include adjustments and static assertions
FairGeoInterface->>FairGeoInterface: Initialize members, add ls method
FairGeoLoader->>FairGeoLoader: Add ls method
FairGeoSet->>FairGeoSet: Destructor handles non-owning TList
This diagram illustrates the primary interactions and changes introduced in this update, focusing on the new methods and adjustments in logging, destructors, and includes. Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
4b2da0c
to
b802e4a
Compare
9f31cd6
to
cf312c2
Compare
FairGeoSet has non-owning references inside a TList to FairGeoNodes. When the dtor of FairGeoSet is called, those FairGeoNodes might already have been destroyed. Calling `Clear` with `"nodelete"` will just remove all the references without touching them.
cf312c2
to
332e015
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: 1
if (volumes && !volumes->IsOwner()) { | ||
// destructing a TList that doesn't own its contents | ||
// still touches the objects inside, even if they're | ||
// already destroeyed |
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.
Correct typo in the comment.
- // already destroeyed
+ // already destroyed
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// already destroeyed | |
// already destroyed |
See: #1514
Checklist:
Summary by CodeRabbit
New Features
ls
method to list objects inFairGeoInterface
andFairGeoLoader
.Bug Fixes
test-splitMQ.sh.in
.Chores
Improvements
FairGeoSet
for non-owningTList
objects.