Vojtech Szocs has posted comments on this change.
Change subject: core: Locale docs servlet.
......................................................................
Patch Set 16: (6 inline comments)
Minor comments mostly, otherwise looks good! Thanks for addressing all my
previous review comments.
....................................................
File
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsServlet.java
Line 16: import org.ovirt.engine.core.utils.ServletUtils;
Line 17:
Line 18: /**
Line 19: * This class serves locale dependent documents. It takes the current
selected locale
Line 20: * and finds the appropriate file based on that locale and the file
requested and
Minor thing: please remove trailing whitespace (TWS)
Line 21: * returns it the browser.
Line 22: */
Line 23: public class DocsServlet extends FileServlet {
Line 24: // The log:
Line 157: final URI refererURL;
Line 158: String result = null;
Line 159: try {
Line 160: String referer = request.getHeader(REFERER);
Line 161: if (null != referer) {
Minor thing: "referer != null"
Line 162: refererURL = new URI(referer);
Line 163: String query = refererURL.getQuery();
Line 164: if (query != query) {
Line 165: String[] parameters = query.split("&");
Line 160: String referer = request.getHeader(REFERER);
Line 161: if (null != referer) {
Line 162: refererURL = new URI(referer);
Line 163: String query = refererURL.getQuery();
Line 164: if (query != query) {
Shouldn't this be "query != null" ?
Line 165: String[] parameters = query.split("&");
Line 166: for (int i = 0; i < parameters.length; i++) {
Line 167: String[] keyValues = parameters[i].split("=");
Line 168: if (LOCALE.equalsIgnoreCase(keyValues[0])) {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocaleUtils.java
Line 16: return getLocaleFromString(localeString, false);
Line 17: }
Line 18:
Line 19: /**
Line 20: * Returns the {@code Locale} based on the passed in string.
Returns the
Minor thing: please remove TWS
Line 21: * default {@code Locale} if a locale cannot be found and the
passed in
Line 22: * flag is set to true.
Line 23: * @param localeString The string to find the {@code Locale} in.
Line 24: * @param returnNull If {@code true} return Locale.US if a locale
cannot be
....................................................
File pom.xml
Line 87: <!-- Plugin Versions -->
Line 88:
<maven-compiler-plugin.version>2.3.2</maven-compiler-plugin.version>
Line 89: <gwt.plugin.version>2.3.0</gwt.plugin.version>
Line 90: <test-jar.plugin.version>2.2</test-jar.plugin.version>
Line 91:
<jboss-modules.plugin.version>1.0-SNAPSHOT</jboss-modules.plugin.version>
Good thing to have these, but please update pluginManagement section to use
them too :)
Note that "test-jar.plugin.version" should actually be
"maven-jar-plugin.version" as test-jar is only a goal of maven-jar-plugin. For
other properties, I'd recommend following "<artifactId>.version" convention,
but it's not that big deal.
Line 92: <javax.jstl.api.version>1.0.3.Final</javax.jstl.api.version>
Line 93: </properties>
Line 94: <dependencyManagement>
Line 95: <dependencies>
Line 88:
<maven-compiler-plugin.version>2.3.2</maven-compiler-plugin.version>
Line 89: <gwt.plugin.version>2.3.0</gwt.plugin.version>
Line 90: <test-jar.plugin.version>2.2</test-jar.plugin.version>
Line 91:
<jboss-modules.plugin.version>1.0-SNAPSHOT</jboss-modules.plugin.version>
Line 92: <javax.jstl.api.version>1.0.3.Final</javax.jstl.api.version>
"javax.jstl.api.version" is already defined above, please remove it here
Line 93: </properties>
Line 94: <dependencyManagement>
Line 95: <dependencies>
Line 96: <dependency>
--
To view, visit http://gerrit.ovirt.org/10065
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I37156061cbdd7d2df3290c88dee933c41e0087c5
Gerrit-PatchSet: 16
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
