Alon Bar-Lev has posted comments on this change.

Change subject: tools: Don't use console for debug log
......................................................................


Patch Set 1: (5 inline comments)

Opps... these where on change 1

....................................................
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/config/EngineConfig.java
Line 9:  */
Line 10: public class EngineConfig {
Line 11:     // The log:
Line 12:     private static final Logger log = 
Logger.getLogger(EngineConfig.class);
Line 13:     
space
Line 14:     // The console:
Line 15:     private static final Console console = Console.getInstance();
Line 16: 
Line 17:     public static final String CONFIG_FILE_PATH_PROPERTY = 
"engine-config.config.file.path";


Line 53:             getInstance().setUpAndExecute(args);
Line 54: 
Line 55:         } catch (Throwable t) {
Line 56:             log.debug("Exiting with error: ", t);
Line 57:             console.writeLine(t.getMessage());
Why not use stderr for errors?
Line 58:             System.exit(1);
Line 59:         }
Line 60:     }
Line 61: 


....................................................
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
Line 183:     private String startUserDialog() throws IOException {
Line 184:         log.debug("starting user dialog.");
Line 185:         String user = null;
Line 186:         while (StringUtils.isBlank(user)) {
Line 187:             console.writeLine("Please enter user: ");
This should go to stderr so redirect will not have this.
Line 188:             user = console.readLine();
Line 189:         }
Line 190:         return user;
Line 191:     }


Line 234:             }
Line 235:             console.write(" ");
Line 236:             console.write("version: ");
Line 237:             console.write(configKey.getVersion());
Line 238:             console.writeLine();
I really recommend of using String.format...
Line 239:         }
Line 240:     }
Line 241: 
Line 242:     /**


....................................................
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/tools/Console.java
Line 19:     public static Console getInstance() {
Line 20:         return instance;
Line 21:     }
Line 22: 
Line 23:     // We need 
Lots of spaces...
Line 24: 
Line 25:     private Console() {
Line 26:         // Nothing, just prevent creation of new instances.
Line 27:     }


--
To view, visit http://gerrit.ovirt.org/12987
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I556e20f6333e22faeb7d212767ebefb99edce54e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to