fgerlits commented on code in PR #1676:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1676#discussion_r1354422474


##########
libminifi/include/core/logging/internal/LogCompressorSink.h:
##########
@@ -54,7 +48,7 @@ class LogCompressorSink : public 
spdlog::sinks::base_sink<std::mutex> {
   void flush_() override;
 
  public:
-  explicit LogCompressorSink(LogQueueSize cache_size, LogQueueSize 
compressed_size, std::shared_ptr<logging::Logger> logger);
+  LogCompressorSink(LogQueueSize cache_size, LogQueueSize compressed_size, 
const std::shared_ptr<logging::Logger>& logger);

Review Comment:
   The detail you are missing is that the `logger` parameter is used twice in 
the constructor: it is passed to the `ActiveCompressor::Allocator` in the 
`compressed_logs_` field, as well as stored directly in the 
`compressor_logger_` field.  I did not like the looks of passing by value, 
copying once and then moving.  I can change it to that if you prefer it.
   
   I removed `explicit` because it doesn't make much sense for constructors 
with multiple parameters, but I can put it back if you think it's useful.



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to