Alon Bar-Lev has posted comments on this change. Change subject: uutils: added new cli parser ......................................................................
Patch Set 11: (4 comments) https://gerrit.ovirt.org/#/c/40157/11/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ArgumentsParser.java File backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ArgumentsParser.java: Line 130: /** Line 131: * Stores default values of every argument, if user don't override it default from this file will be used. Line 132: */ Line 133: private static final Properties defaultProperties = loadProperties( Line 134: ArgumentsParser.class.getResourceAsStream("defaults.properties") question: shouldn't we close this stream? private static final Properties defaultProperties; static { try(InputStream is = ArgumentsParser.class.getResourceAsStream("defaults.properties")) { defaultProperties = loadProperties(is); } } Line 135: ); Line 136: Line 137: /** Line 138: * Stores user defined arguments in properties file. Line 278: * Line 279: * @return list of erros Line 280: */ Line 281: public List<Throwable> getErrors() { Line 282: return errors; I hope you do not kill me, but as this is infra component it is good to know the practices... you should not allow caller to modify collection. return Collections.unmodifiableList(errors); Line 283: } Line 284: Line 285: /** Line 286: * Return map of validated and parsed arguments Line 287: * Line 288: * @return Map of validated and parsed arguments Line 289: */ Line 290: public Map<String, Object> getArgMap() { Line 291: return argMap; same... return Collections.unmodifiableMap(argMap); Line 292: } Line 293: Line 294: /** Line 295: * Go through all arguments which has default value. If such argument is not in {@code argMap} put it there. Line 454: * $prefix.help.footer Line 455: * Line 456: * @return formatted string with usage Line 457: */ Line 458: public String getUsage() { usually it is best to have all public together at bottom or top (depending on taste). Line 459: StringBuilder help = new StringBuilder(String.format("Options:%n")); Line 460: for(String arg : getPrefixArguments()) { Line 461: Argument argument = this.arguments.get(arg); Line 462: help.append( -- To view, visit https://gerrit.ovirt.org/40157 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00042b669e19293641579582223e7ca40717132d Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Mooli Tayer <mta...@redhat.com> Gerrit-Reviewer: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
