[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1586: MINIFICPP-2127 Add multipart upload support for PutS3Object processor

["szaszm (via GitHub)" <gi...@apache.org>]

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


##########
libminifi/include/utils/TimeUtil.h:
##########
@@ -189,6 +189,20 @@ inline bool unit_matches<std::chrono::days>(const std::string& unit) {
   return unit == "d" || unit == "day" || unit == "days";
 }
 
+template<>
+inline bool unit_matches<std::chrono::weeks>(const std::string& unit) {
+  return unit == "w" || unit == "week" || unit == "weeks";
+}
+
+template<>
+inline bool unit_matches<std::chrono::months>(const std::string& unit) {
+  return unit == "mon" || unit == "month" || unit == "months";
+}
+
+template<>
+inline bool unit_matches<std::chrono::years>(const std::string& unit) {
+  return unit == "y" || unit == "year" || unit == "years";
+}

Review Comment:
   nifi equivalent: https://github.com/apache/nifi/blob/main/nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/FormatUtils.java#L45
   
   We should try to match that for maximum compatibility. I don't really mind if we accept/support more, but if a unit is supported, then we shouldn't accept less. And if NiFi ever decides to use other suffixes for the things that are not there yet (microsec/mon/year), then it becomes an awkward backwards compatibility vs NiFi compatibility question.



-- 
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