leventov commented on a change in pull request #6629: Add support parallel
combine in brokers
URL: https://github.com/apache/incubator-druid/pull/6629#discussion_r243355031
##########
File path: core/src/main/java/org/apache/druid/java/util/common/DateTimes.java
##########
@@ -99,6 +99,11 @@ public static DateTime of(String instant)
return new DateTime(instant, ISOChronology.getInstanceUTC());
}
+ public static DateTime of(String format, Object... formatArgs)
Review comment:
That other method is added recently. It used at nine places in tests. Six of
them are moot, because parameters are constants and instead
`Intervals.of(String)` could be used. At the other three places, I would say,
it would be better to use `new Interval(Datetimes.of(...), Datetimes.of(...))`,
where the `Datetimes.of()` is what is suggested above.
Minor, but real disadvantages of the vararg methods is that they are not
reflected in the corresponding IntelliJ inspection (see "MalformedFormatString"
in inspectionProfiles/Druid.xml) and extra performance cost. While they could
be replaced trivially.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
