mooli tayer has posted comments on this change.

Change subject: extensions test tool: logger
......................................................................


Patch Set 18:

(1 comment)

> (1 comment)
 > 
 > > (1 comment)
 > >
 > > This classes would be more usable to me if they would only throw
 > > exception if something goes wrong, and not if parsing fails.
 > > maybe return response containing args, status, msg and drop
 > > ExitException (or always return map and have them as keys in map)
 > >
 > > Up to you two..
 > 
 > yes, see my last notes, and write what you think

read back quite a while and I'm not sure what you refer to.

Other then that the classes where very easy and comfortable to use so
thanks!

https://gerrit.ovirt.org/#/c/37886/18/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java
File 
backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java:

Line 52:     public Map<String, Object> parse(String[] args) throws Exception {
Line 53:         return parse(new ArrayList<String>(Arrays.asList(args)));
Line 54:     }
Line 55: 
Line 56:     public Map<String, Object> parse(List<String> args) throws 
Exception {
why do these two need to throw exception?
Line 57:         List<Object> others = new ArrayList<>();
Line 58:         Map<String, Object> argMap = new HashMap<>();
Line 59:         args.addAll(0, defaults);
Line 60: 


-- 
To view, visit https://gerrit.ovirt.org/37886
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie06113c5d56a49e58d557c851f9ff00b9a9ca409
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ondřej Macháček <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ondra Machacek <[email protected]>
Gerrit-Reviewer: Ondřej Macháček <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to