szaszm commented on a change in pull request #721: MINIFICPP-1135 - Add a watchdog to schedulingAgent to warn in case of… URL: https://github.com/apache/nifi-minifi-cpp/pull/721#discussion_r373631259
########## File path: libminifi/include/SchedulingAgent.h ########## @@ -39,6 +41,9 @@ #include "core/controller/ControllerServiceProvider.h" #include "core/controller/ControllerServiceNode.h" +#define SCHEDULING_WATCHDOG_CHECK_PERIOD 1000 // msec +#define SCHEDULING_WATCHDOG_ALERT_DEFAULT_PERIOD 5000 // msec Review comment: I prefer to have the unit in the name, i.e. `SCHEDULING_WATCHDOG_ALERT_DEFAULT_PERIOD_MS`. If the unit can not be in the type, having it in the name is still more helpful than just comments. For examples, see: https://doc.qt.io/qt-5/qthread.html#wait Bad example: wait(). One needs to read the class docs to figure out whether they should pass 5 or 5000 to the function. Good example: sleep(). The unit is in the name, therefore usage is clear from the function prototype(, which usually contains parameter names). Other: I'd move DEFAULT in front of ALERT to make the name sound more like English. ---------------------------------------------------------------- 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
