Skip to content

Conversation

@damien-robotsix
Copy link
Contributor

Related to
#39 (comment)

Probably not the best way to solve it but it works for my application.

It might be a base to solve the issue.

@henrygerardmoore henrygerardmoore self-requested a review November 6, 2024 17:28
@damien-robotsix
Copy link
Contributor Author

Hello @henrygerardmoore can I ping you about this PR?

@henrygerardmoore
Copy link
Collaborator

Hello @henrygerardmoore can I ping you about this PR?

Yes sorry, let me review it this weekend 👍

@henrygerardmoore henrygerardmoore mentioned this pull request Jun 28, 2025
@henrygerardmoore
Copy link
Collaborator

@damien-robotsix I made the branch add_custom_container_tests to add tests (even though they're basically just static asserts 😄). I mostly made this to make sure I understand the changes, could you take a look at that? I made a draft PR here (#56) just for easy reviewing, but feel free to merge those changes into this branch and we can continue to work off of this PR.

I think this shouldn't break any interfaces, correct? Unless, I suppose, someone was relying on the incorrect container overload for a custom type? Is that your understanding too?

In any case, thank you for the PR!

@henrygerardmoore
Copy link
Collaborator

henrygerardmoore commented Jul 2, 2025

@damien-robotsix does this seem correct to you? If so and if the merge conflict is resolved, I think this PR seems good to merge

I think this shouldn't break any interfaces, correct? Unless, I suppose, someone was relying on the incorrect container overload for a custom type? Is that your understanding too?

@damien-robotsix
Copy link
Contributor Author

I just rebased on current main to allow merge

@henrygerardmoore henrygerardmoore merged commit fc2e898 into PickNikRobotics:main Jul 9, 2025
6 checks passed
@damien-robotsix damien-robotsix deleted the custom_container_handling branch July 21, 2025 08:56
@damien-robotsix
Copy link
Contributor Author

@henrygerardmoore Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants