Copilot commented on code in PR #525:
URL: https://github.com/apache/pulsar-client-cpp/pull/525#discussion_r2577461479
##########
lib/ProducerImpl.cc:
##########
@@ -974,14 +974,14 @@ bool ProducerImpl::encryptMessage(proto::MessageMetadata&
metadata, SharedBuffer
encryptedPayload);
}
-void ProducerImpl::disconnectProducer(const boost::optional<std::string>&
assignedBrokerUrl) {
+void ProducerImpl::disconnectProducer(const optional<std::string>&
assignedBrokerUrl) {
LOG_INFO("Broker notification of Closed producer: "
- << producerId_ << (assignedBrokerUrl ? (" assignedBrokerUrl: " +
assignedBrokerUrl.get()) : ""));
+ << producerId_ << (assignedBrokerUrl ? (" assignedBrokerUrl: " +
*assignedBrokerUrl) : ""));
resetCnx();
scheduleReconnection(assignedBrokerUrl);
}
-void ProducerImpl::disconnectProducer() { disconnectProducer(boost::none); }
+void ProducerImpl::disconnectProducer() { disconnectProducer(nullptr); }
Review Comment:
`nullptr` is not implicitly convertible to `std::optional<std::string>` in
C++17. Use `{}` or `std::nullopt` instead.
```cpp
void ProducerImpl::disconnectProducer() { disconnectProducer({}); }
```
```suggestion
void ProducerImpl::disconnectProducer() { disconnectProducer({}); }
```
##########
lib/HandlerBase.cc:
##########
@@ -86,18 +85,18 @@ void HandlerBase::setCnx(const ClientConnectionPtr& cnx) {
connection_ = cnx;
}
-void HandlerBase::grabCnx() { grabCnx(boost::none); }
+void HandlerBase::grabCnx() { grabCnx(nullptr); }
Review Comment:
`nullptr` is not implicitly convertible to `std::optional<std::string>` in
C++17. Use `{}` or `std::nullopt` instead.
```cpp
void HandlerBase::grabCnx() { grabCnx({}); }
```
```suggestion
void HandlerBase::grabCnx() { grabCnx({}); }
```
##########
lib/Commands.cc:
##########
@@ -34,6 +34,7 @@
#include "OpSendMsg.h"
#include "PulsarApi.pb.h"
#include "Url.h"
+#include "boost/throw_exception.hpp"
Review Comment:
This include appears to be unused and seems contradictory to the goal of
reducing Boost dependencies. Consider removing it if it's not needed.
```suggestion
```
##########
lib/ConsumerImpl.cc:
##########
@@ -1313,11 +1313,11 @@ void ConsumerImpl::negativeAcknowledge(const MessageId&
messageId) {
negativeAcksTracker_->add(messageId);
}
-void ConsumerImpl::disconnectConsumer() { disconnectConsumer(boost::none); }
+void ConsumerImpl::disconnectConsumer() { disconnectConsumer(nullptr); }
Review Comment:
`nullptr` is not implicitly convertible to `std::optional<std::string>` in
C++17. Use `{}` or `std::nullopt` instead.
```cpp
void ConsumerImpl::disconnectConsumer() { disconnectConsumer({}); }
```
```suggestion
void ConsumerImpl::disconnectConsumer() { disconnectConsumer({}); }
```
##########
lib/HandlerBase.cc:
##########
@@ -177,8 +176,8 @@ void HandlerBase::handleDisconnection(Result result, const
ClientConnectionPtr&
break;
}
}
-void HandlerBase::scheduleReconnection() { scheduleReconnection(boost::none); }
-void HandlerBase::scheduleReconnection(const boost::optional<std::string>&
assignedBrokerUrl) {
+void HandlerBase::scheduleReconnection() { scheduleReconnection(nullptr); }
Review Comment:
`nullptr` is not implicitly convertible to `std::optional<std::string>` in
C++17. Use `{}` or `std::nullopt` instead.
```cpp
void HandlerBase::scheduleReconnection() { scheduleReconnection({}); }
void HandlerBase::scheduleReconnection(const optional<std::string>&
assignedBrokerUrl) {
```
```suggestion
void HandlerBase::scheduleReconnection() {
scheduleReconnection(std::nullopt); }
```
##########
lib/ClientConnection.h:
##########
@@ -37,8 +37,6 @@
#include <boost/asio/ssl/stream.hpp>
#include <boost/asio/strand.hpp>
#endif
Review Comment:
Missing `#include <any>` header. The file uses `std::any` on line 378 but
doesn't include the required header.
Add after line 39:
```cpp
#include <any>
```
```suggestion
#endif
#include <any>
```
--
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]
