Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix multiple-definition error in Frame.h #369

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

nathanwbrei
Copy link
Contributor

@nathanwbrei nathanwbrei commented Feb 6, 2023

Frame::Frame(), get(), put(), and putParameter() methods had (non-templated) definitions in the header file. This caused errors when linking multiple translation units that each imported Frame.h. These definitions have now been specified as inline.

BEGINRELEASENOTES

  • Mark non-templated definitions of Frame::Frame, Frame::get, Frame::put and Frame::putParameters as inline to fix linker errors.

ENDRELEASENOTES

Frame::Frame(), get(), put(), and putParameter() methods had (non-templated) definitions in the header file. These have now been specified as inline.
Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this. Looks like we haven't been using this on large enough scales yet.

Do you need a new tag as well, or are you working off the main branch in any case?

@tmadlener tmadlener merged commit 811aca9 into AIDASoft:master Feb 6, 2023
@nathanwbrei
Copy link
Contributor Author

Thanks Thomas! That was very fast.
I'm working off of my own fork so that I can make changes quickly and not be bothering you all the time. If I find more bugs, I'll put each in its own separate, small PR. There's no need to go through the trouble of creating a new tag just yet. I'm doing super rapid development with the goal of cutting a new JANA release by next Wednesday that I can share with the rest of the EIC collaboration. I think it would make the most sense to cut a new podio release slightly before then, once I'm confident it does everything necessary for now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants