Skip to content

Detect proto messages based on presence of DebugString.#2837

Closed
inazarenko wants to merge 3 commits intogoogle:masterfrom
inazarenko:duck_type_protos
Closed

Detect proto messages based on presence of DebugString.#2837
inazarenko wants to merge 3 commits intogoogle:masterfrom
inazarenko:duck_type_protos

Conversation

@inazarenko
Copy link
Copy Markdown
Contributor

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

Second attempt, since #2818 was rejected.

@inazarenko
Copy link
Copy Markdown
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
Copy Markdown

@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
Copy Markdown
Contributor Author

Gentle ping :)

template <typename T>
struct IsAProtocolMessage
: public std::is_convertible<const T*, const ::proto2::MessageLite*> {};
class IsAProtocolMessage {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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