[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1320: MINIFICPP-1813 Stabilize AbsoluteTimeoutTest in CI on Windows

[GitBox <gi...@apache.org>]

szaszm commented on code in PR #1320:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1320#discussion_r860797382


##########
extensions/http-curl/tests/AbsoluteTimeoutTest.cpp:
##########
@@ -22,20 +22,14 @@
 #include "tests/TestServer.h"
 #include "HTTPHandlers.h"
 
+using namespace std::literals::chrono_literals;
+
 int main() {
   TestController controller;
 
   std::string port = "12324";
   std::string rootURI =  "/";
-  TimeoutingHTTPHandler handler({
-    std::chrono::milliseconds(500),
-    std::chrono::milliseconds(500),
-    std::chrono::milliseconds(500),
-    std::chrono::milliseconds(500),
-    std::chrono::milliseconds(500),
-    std::chrono::milliseconds(500),
-    std::chrono::milliseconds(500)
-  });
+  TimeoutingHTTPHandler handler({35, 100ms});

Review Comment:
   I would change this to have explicit type with parens instead of braces. Usually I prefer this style, but in this case, I want to make it clear that this is not the initializer_list contstructor of std::vector, but it's going to create 35 elements with the value 100ms. I got bitten by a similar thing when I tried to direct-list-initialize a string with N elements of a certain character, but it got converted to char and used the initializer_list constructor instead of the two parameter one. After changing to parens, it used the two parameter ctor.
   ```suggestion
     TimeoutingHTTPHandler handler(std::vector<std::chrono::milliseconds>(35, 100ms));
   ```



-- 
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-unsubscribe@nifi.apache.org

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