Martin Mucha has posted comments on this change. Change subject: userportal,webadmin:change Translator to interface ......................................................................
Patch Set 8: (5 comments) http://gerrit.ovirt.org/#/c/26596/8/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/EnumTranslator.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/EnumTranslator.java: Line 22: Line 23: @Override Line 24: public String translate(Enum<?> key) { Line 25: if(key == null) { Line 26: logger.log(Level.INFO, "trying to localize null, probable error. " + > This could be logged as a WARNING instead of INFO. Done Line 27: "Exception is not thrown, returning '"+ constants.notAvailableLabel()+"'", new RuntimeException()); Line 28: return constants.notAvailableLabel(); Line 29: } Line 30: Line 23: @Override Line 24: public String translate(Enum<?> key) { Line 25: if(key == null) { Line 26: logger.log(Level.INFO, "trying to localize null, probable error. " + Line 27: "Exception is not thrown, returning '"+ constants.notAvailableLabel()+"'", new RuntimeException()); > Please format the code to comply with general conventions (i.e. space befor well, I'd expect that enum translation is used on many places, and method argument can be calculated (especially in some majestic switch) and if that calculation returns null, it won't be that easy to find out who produces that null. But ok, you can put there a conditional breakpoint. - changed to WARNING - formatted - removed RuntimeException - removed code duplicity "constants.notAvailableLabel()" Line 28: return constants.notAvailableLabel(); Line 29: } Line 30: Line 31: try { Line 41: return notLocalizedKey(key, null); Line 42: } Line 43: Line 44: private String notLocalizedKey(Enum<?> key, MissingResourceException e) { Line 45: String logString = "Missing Enum resource '" + key + "'.";//$NON-NLS-1$ > Now there is missing space:) Done Line 46: if (e != null) { Line 47: logString += " " + e.getLocalizedMessage(); Line 48: } Line 49: Line 46: if (e != null) { Line 47: logString += " " + e.getLocalizedMessage(); Line 48: } Line 49: Line 50: logger.info(logString); > This could be logged as a WARNING instead of INFO. Done Line 51: return key.name(); Line 52: } Line 53: Line 54: private String keyToTranslate(Enum<?> key) { Line 52: } Line 53: Line 54: private String keyToTranslate(Enum<?> key) { Line 55: String className = key.getDeclaringClass().toString(); Line 56: String classNameWithoutPackage = className.substring(className.lastIndexOf(".")+1,className.length()); //$NON-NLS-1$ > Please format the code to comply with general conventions (i.e. space befor Done Line 57: Line 58: return classNameWithoutPackage + "___" + key.name(); Line 59: } Line 60: -- To view, visit http://gerrit.ovirt.org/26596 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib018c1faf0c2e1ebaa81217d5e3696d9c8de20cf Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [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
