Vojtech Szocs has posted comments on this change.
Change subject: core: rearrange product uris
......................................................................
Patch Set 14:
(3 comments)
Just two minor comments, otherwise an excellent patch and move into right
direction, in my opinion.
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ForwardServlet.java
Line 36: * the path in the target servlet context. So if the ForwardServlet is
mapped to /docs and the target context
Line 37: * is /ovirt-engine/docs. Then /docs/something gets forwarded to
/ovirt-engine/docs/something. <br />
Line 38: * <br />
Line 39: * The targetContext param value can contain property expressions for
expanding Engine property values in
Line 40: * form of %{PROP_NAME}.
Excellent Javadoc.
Line 41: */
Line 42: public class ForwardServlet extends HttpServlet {
Line 43: /**
Line 44: * The logger instance.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GwtDynamicHostPageServlet.java
Line 174: * @return
Line 175: */
Line 176: private String getConfiguration(final HttpServletRequest request)
{
Line 177: ObjectNode node = mapper.createObjectNode();
Line 178: node.put(MD5Attributes.ATTR_BASE_CONTEXT_PATH.getKey(),
ServletUtils.getBaseContextPath(request));
Just a minor thing, this results in following data embedded in host page:
var baseContextPath = { "baseContextPath": "..." };
Maybe we can change it to something like:
var baseContextPath = { "value": "..." };
What do you think? i.e. having something like:
node.put("value", ServletUtils.getBaseContextPath(request));
Line 179: return node.toString();
Line 180: }
Line 181:
Line 182: /**
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/BaseContextPathData.java
Line 14: return $wnd.baseContextPath;
Line 15: }-*/;
Line 16:
Line 17: private native String getValue() /*-{
Line 18: return this.baseContextPath;
Minor thing, see my comment in GwtDynamicHostPageServlet.java, this could be:
return this.value;
By the way, getValue method can be safely marked as public, not sure we need an
extra getPath method.
Line 19: }-*/;
Line 20:
Line 21: public String getPath() {
Line 22: return getValue();
--
To view, visit http://gerrit.ovirt.org/20473
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cb4822f6bf4d372715e12858635db5ed3edd115
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yaniv Dary <[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
