szaszm commented on a change in pull request #741: MINIFICPP-1139 Implemented. URL: https://github.com/apache/nifi-minifi-cpp/pull/741#discussion_r383920888
########## File path: extensions/windows-event-log/ConsumeWindowsEventLog.h ########## @@ -101,14 +100,11 @@ class ConsumeWindowsEventLog : public core::Processor protected: - bool subscribe(const std::shared_ptr<core::ProcessContext> &context); - void unsubscribe(); - int processQueue(const std::shared_ptr<core::ProcessSession> &session); + void putEventRenderFlowFileToSession(const EventRender& eventRender, core::ProcessSession& session); wel::WindowsEventLogHandler getEventLogHandler(const std::string & name); bool insertHeaderName(wel::METADATA_NAMES &header, const std::string &key, const std::string &value); void LogWindowsError(); - void processEvent(EVT_HANDLE eventHandle); - bool processEventsAfterBookmark(EVT_HANDLE hEventResults, const std::wstring& channel, const std::wstring& query); + bool createEventRender(EVT_HANDLE eventHandle, EventRender& eventRender); Review comment: A small suggestion: "Throw an exception to signal that a function can’t perform its assigned task" (instead of returning success/failure `bool`) This way `eventRender` can become the return value, making the function more intuitive to use. http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e2-throw-an-exception-to-signal-that-a-function-cant-perform-its-assigned-task ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
