Ondřej Macháček has posted comments on this change.

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


Patch Set 13:

(5 comments)

https://gerrit.ovirt.org/#/c/37886/13/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ExtensionsToolExecutor.java
File 
backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ExtensionsToolExecutor.java:

Line 69:         } catch (Throwable t) {
Line 70:             String message = t.getMessage() != null ? t.getMessage() : 
t.getClass().getName();
Line 71:             logger.error(message);
Line 72:             logger.debug(message, t);
Line 73:             t.printStackTrace();
> this is done automatically in debug, no?
oh, yes. :) will remove..
Line 74:         }
Line 75:         System.exit(exitStatus);
Line 76:     }
Line 77: 


Line 89:         }
Line 90: 
Line 91:         for(String proxy : proxies.keySet()) {
Line 92:             extensionsManager.initialize(proxy);
Line 93:             proxies.put(proxy, 
extensionsManager.getExtensionByName(proxy));
> use entrySet() and setValue()?
ok.
Line 94:         }
Line 95:     }
Line 96: 
Line 97:     private static Map<String, ModuleService> loadModules(Class cls) {


Line 115:             throw new ExitException("Please provide module.", 0);
Line 116:         }
Line 117: 
Line 118:         return new ParametersParser(
Line 119:             
ExtensionsToolExecutor.class.getResourceAsStream("arguments.properties.in")
> .in should not be in jar, it is template for makefile.
ok
Line 120:         ).parse(args);
Line 121:     }
Line 122: 
Line 123:     private static void setupLogger(Map<String, Object> args) throws 
IOException{


https://gerrit.ovirt.org/#/c/37886/13/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 50:     }
Line 51: 
Line 52:     public Map<String, Object> parse(String[] args, boolean isModule) 
throws Exception {
Line 53:         return parse(new ArrayList<String>(Arrays.asList(args)), 
isModule);
Line 54:     }
> I do not understand why we have a difference between module or not, please 
In first parsing(core) we just parse until we hit module (ie. logger, aaa, 
etc). When we hit the module, we add every other argument to __others__.

In second parsing it's different. When we hit action (parameter without '--' 
prefix) we need to continue and parse all arguments. This is different 
behaviour which I handle with isModule boolean.
Line 55: 
Line 56:     public Map<String, Object> parse(List<String> args, boolean 
isModule) throws Exception {
Line 57:         String usage = "core";
Line 58:         List<Object> others = new ArrayList<>();


Line 265:     }
Line 266: 
Line 267:     public static Properties loadProperties(InputStream resource) {
Line 268:         try (
Line 269:             Reader is = new InputStreamReader(resource, "UTF-8");
> what did you revert the Charset.forName()?
I don't know how did I do that... :( I'll fix it.
Line 270:         ) {
Line 271:             Properties prop = new Properties();
Line 272:             prop.load(is);
Line 273:             return prop;


-- 
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: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ondřej Macháček <machacek.on...@gmail.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Ondra Machacek <omach...@redhat.com>
Gerrit-Reviewer: Ondřej Macháček <machacek.on...@gmail.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to