gianm commented on a change in pull request #6027: Make Parser.parseToMap() to
return a mutable Map
URL: https://github.com/apache/incubator-druid/pull/6027#discussion_r206743451
##########
File path:
java-util/src/main/java/io/druid/java/util/common/parsers/Parser.java
##########
@@ -40,10 +40,13 @@ default void startFileFromBeginning()
/**
* Parse a String into a Map. The result can be null which means the given
input string will be ignored.
*
+ * This method is defined to return LinkedHashMap instead of just Map to
guarantee the mutability (that is needed
+ * for https://github.com/apache/incubator-druid/pull/6027) and the order of
the entries.
+ *
* @throws ParseException if the String cannot be parsed
*/
@Nullable
- Map<K, V> parseToMap(String input);
+ LinkedHashMap<K, V> parseToMap(String input);
Review comment:
Other comments aside, I don't think we should change this signature in a
patch release. `Parser` is returned by `ParseSpec` which is an extension point.
I think it's an oversight that Parser isn't yet tagged with ExtensionPoint too.
----------------------------------------------------------------
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]
