BewareMyPower commented on code in PR #148:
URL: https://github.com/apache/pulsar-client-cpp/pull/148#discussion_r1052081373
##########
include/pulsar/Message.h:
##########
@@ -188,8 +188,7 @@ class PULSAR_PUBLIC Message {
MessageImplPtr impl_;
Message(MessageImplPtr& impl);
- Message(const proto::CommandMessage& msg, proto::MessageMetadata& data,
SharedBuffer& payload,
- int32_t partition);
+ Message(const MessageId& messageId, proto::MessageMetadata& metadata,
SharedBuffer& payload);
Review Comment:
Just have a thought, not a requested change and this PR needs no change
about this comment. I think the constructor of `Message` should have only one
private constructor.
```c++
Message(const MessageImplPtr& impl);
```
The other constructors should be moved to `MessageImpl`. After that, there
is no need to expose the classes (even if as the forward declarations) in the
header.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
