Alexander Wels has posted comments on this change. Change subject: userportal,webadmin: messages locale unit test ......................................................................
Patch Set 2: (4 comments) http://gerrit.ovirt.org/#/c/32995/2/frontend/webadmin/modules/gwt-common/src/test/java/org/ovirt/engine/ui/common/CommonApplicationMessagesTest.java File frontend/webadmin/modules/gwt-common/src/test/java/org/ovirt/engine/ui/common/CommonApplicationMessagesTest.java: Line 58: } Line 59: return builder.toString(); Line 60: } Line 61: Line 62: protected void compareMethodsToProperties(Method[] messagesMethods, Properties properties, File propertiesFile) { > Just a small thing, I see that "File propertiesFile" is passed around only Done Line 63: for (Method method: messagesMethods) { Line 64: int count = 0; Line 65: checkForMissingDefault(properties, propertiesFile, method); Line 66: checkPlaceHolders(properties, propertiesFile, method, count); Line 66: checkPlaceHolders(properties, propertiesFile, method, count); Line 67: } Line 68: } Line 69: Line 70: private void checkPlaceHolders(Properties properties, File propertiesFile, Method method, int count) { > I assume "count" is the starting placeholder index here? I think I had an Integer at first for something but refactored it away. Line 71: if (properties.getProperty(method.getName()) != null) { Line 72: Set<Integer> foundIndex = new HashSet<Integer>(); Line 73: Set<Integer> requiredIndexes = determineRequiredIndexes(method.getParameterAnnotations()); Line 74: int minRequired = requiredIndexes.size(); Line 148: * @param path The path to the directory. Line 149: * @return An {@code Array} of {@code File} objects. Line 150: * @throws URISyntaxException If path doesn't exist Line 151: */ Line 152: private File[] getMessagesPropertiesFiles(final String path) throws URISyntaxException { > Hm, we could accept "Class<? extends Messages> messagesClass" argument here Done Line 153: File currentDir = new File( Line 154: this.getClass().getResource(path).toURI().toASCIIString().replaceAll("file:", "")); Line 155: return currentDir.listFiles(new FilenameFilter() { Line 156: http://gerrit.ovirt.org/#/c/32995/2/frontend/webadmin/modules/userportal-gwtp/src/test/java/org/ovirt/engine/ui/userportal/ApplicationMessagesTest.java File frontend/webadmin/modules/userportal-gwtp/src/test/java/org/ovirt/engine/ui/userportal/ApplicationMessagesTest.java: Line 16: super.setUp(); Line 17: List<Method> allMessageMethods = new ArrayList<Method>(); Line 18: allMessageMethods.addAll(Arrays.asList(messagesMethods)); Line 19: allMessageMethods.addAll(Arrays.asList(ApplicationMessages.class.getDeclaredMethods())); Line 20: messagesMethods = allMessageMethods.toArray(new Method[allMessageMethods.size()]); > I think this code could be a lot simpler if we avoid using Java arrays, for Done Line 21: } -- To view, visit http://gerrit.ovirt.org/32995 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icda389f2ca08e9d3b7515bb4a0dc1213bc91aeef Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
