Vojtech Szocs has posted comments on this change.
Change subject: frontend: Non-plugin automatic invocation of console session
......................................................................
Patch Set 4: (1 inline comment)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/uicommon/SpiceNativeImpl.java
Line 21:
Line 22:
configBuilder.append(lineSeparator).append("type=spice")//$NON-NLS-1$
Line 23:
.append(lineSeparator).append("host=").append(getHost())//$NON-NLS-1$
Line 24:
.append(lineSeparator).append("password=").append(getPassword())//$NON-NLS-1$
Line 25:
.append(lineSeparator).append("port=").append(Integer.toString(getPort()));//$NON-NLS-1$
I agree with Alon that code with StringBuilder is far less readable than code
with String.format, but considering String.format performance in JavaScript, we
typically stick to using StringBuilder in such cases.
Another thing we could do is use some kind of cached template that could be
applied on specific values without having to parse pattern each time when
needed (unlike String.fromat which parses the pattern immediately before
applying specific values). We could use GWT Messages for this purpose, or
implement our own templating that would internally use StringBuilder for better
performance.
For now, I guess we can live with StringBuilder :-)
Line 26:
Line 27:
ConsoleModel.makeConsoleConfigRequest(configBuilder.toString());
Line 28: }
Line 29:
--
To view, visit http://gerrit.ovirt.org/11703
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib31e870deb7ecb3dac4cff25e49a3ebca4706a25
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
