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

Detect proto messages based on presence of DebugString. #2837

Closed
wants to merge 3 commits into from
Closed

Detect proto messages based on presence of DebugString. #2837

wants to merge 3 commits into from

Conversation

inazarenko
Copy link
Contributor

fixes #2817 (allows open-source proto messages to be printed using DebugString).

Second attempt, since #2818 was rejected.

@inazarenko
Copy link
Contributor Author

Note that this required more metaprogramming magic that I'm really comfortable with, so I'm prepared for a few round-trips of feedback before this is ready to merge. Please let me know if this is the approach you had in mind @asoffer

@asoffer asoffer self-assigned this May 12, 2020
Copy link

@kaiwalyakoparkar kaiwalyakoparkar left a comment

Choose a reason for hiding this comment

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

These changes should be merged

@inazarenko
Copy link
Contributor Author

Gentle ping :)

template <typename T>
struct IsAProtocolMessage
: public std::is_convertible<const T*, const ::proto2::MessageLite*> {};
class IsAProtocolMessage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're no longer checking for being a protocol message but rather for DebugString and ShortDebugString, can we change the name to HasDebugStringAndShortDebugString?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Should I do it here, or will you pull the changes in through Google's internal source control?

@mbxx mbxx mentioned this pull request Oct 8, 2020
mbxx added a commit that referenced this pull request Oct 8, 2020
mbxx added a commit that referenced this pull request Oct 8, 2020
derekmauro added a commit that referenced this pull request Oct 13, 2020
derekmauro added a commit that referenced this pull request Oct 13, 2020
derekmauro added a commit that referenced this pull request Oct 13, 2020
derekmauro added a commit that referenced this pull request Oct 14, 2020
derekmauro added a commit that referenced this pull request Oct 14, 2020
derekmauro added a commit that referenced this pull request Oct 14, 2020
@asoffer
Copy link
Contributor

asoffer commented Nov 2, 2020

@inazarenko this should be working now though as you can see there are merge conflicts because we made a few tweaks to the implementation. Leaving this for you to close.

@asoffer asoffer assigned inazarenko and unassigned asoffer Nov 2, 2020
@inazarenko inazarenko closed this Nov 2, 2020
@inazarenko inazarenko deleted the duck_type_protos branch November 2, 2020 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proto messages from open source protobufs are printed as bytes.
4 participants