Skip to content
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

[QC-1176] Display a warning for developers if a registered object is not mergeable #2400

Merged
merged 6 commits into from
Nov 25, 2024

Conversation

justonedev1
Copy link
Collaborator

this is just a draft with an idea, how to do it compile time, I am not sure, that you like it. It does not build, it would need a bit more work. Otherwise I don't know how to do this compile time and it would need to be done run time via dynamic_cast

@justonedev1 justonedev1 changed the title [QC-1176] Display a warning for developers if a registered object is not mergeable WIP [QC-1176] Display a warning for developers if a registered object is not mergeable Aug 28, 2024
@justonedev1 justonedev1 changed the title WIP [QC-1176] Display a warning for developers if a registered object is not mergeable [WIP] [QC-1176] Display a warning for developers if a registered object is not mergeable Aug 28, 2024
@knopers8
Copy link
Collaborator

For the record, we discussed in person, the agreed improvements are:

  • move the type handling logic to Mergers
  • find a way to avoid including all of the complete types that are checked (typically, the user will include what they need)
  • fail the compilation if the type is not mergeable, but add another method like "startPublishingNotMergeable" to allow expert users to do what they want.

@justonedev1
Copy link
Collaborator Author

I think that I addressed everything that we talked about, but a bit different way. If you would prefer something else, I can do that.

Copy link
Collaborator

@knopers8 knopers8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, just some details to sort out, but in general the idea is good.

Do I understand correctly that the inclusion order of ObjectManager.h and PostProcessingInterface.h should not matter because the template is put in place after both?

Framework/src/ObjectsManager.cxx Outdated Show resolved Hide resolved
Framework/src/ObjectsManager.cxx Outdated Show resolved Hide resolved
Copy link
Collaborator

@knopers8 knopers8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, it seems all good now!

@justonedev1 justonedev1 changed the title [WIP] [QC-1176] Display a warning for developers if a registered object is not mergeable [QC-1176] Display a warning for developers if a registered object is not mergeable Nov 21, 2024
@justonedev1 justonedev1 marked this pull request as ready for review November 21, 2024 13:11
@justonedev1
Copy link
Collaborator Author

@wiechula Can you please take a look at this PR?

Modules/TPC/src/PID.cxx Outdated Show resolved Hide resolved
@justonedev1 justonedev1 disabled auto-merge November 25, 2024 08:59
@justonedev1 justonedev1 merged commit 92fe168 into AliceO2Group:master Nov 25, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants