Tomas Jelinek has posted comments on this change.
Change subject: frontend: Make VNC implementation configurable from UI
......................................................................
Patch Set 1: (3 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java
Line 57: KeyMaker keyMaker = new KeyMaker(id, context);
Line 58:
Line 59: clientStorage.setLocalItem(keyMaker.make(SELECTED_PROTOCOL),
selectedProtocol.toString());
Line 60:
Line 61: switch (selectedProtocol) {
I would go with if-else as this is NPE vulnerable. But if you want to use a
switch-case then please add a null check before it and return if he
selectedProtocol is null.
Line 62: case SPICE:
Line 63: storeSpiceData(model, keyMaker);
Line 64: break;
Line 65: case VNC:
Line 106: }
Line 107:
Line 108: model.setSelectedProtocol(ConsoleProtocol.VNC);
Line 109:
asVncConsoleModel(model).setVncImplementation(VncConsoleModel.ClientConsoleMode
Line 110:
.valueOf(clientStorage.getLocalItem(keyMaker.make(VNC_CLIENT_MODE))));
You can not be sure that the value is present in the local storage (see the
storeVncData). Please wrap the ...valueOf(...) by try-catch
Line 111: }
Line 112:
Line 113: protected RdpConsoleModel asRdpConsoleModel(HasConsoleModel
model) {
Line 114: return (RdpConsoleModel) model.getAdditionalConsoleModel();
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java
Line 263: }
Line 264:
Line 265: public boolean isWebSocketProxyDefined() {
Line 266: String wsConfig = (String)
AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.WebSocketProxy);
Line 267: return wsConfig != null && !"".equals(wsConfig) &&
!"Off".equals(wsConfig); //$NON-NLS-1$ $NON-NLS-2$
does it have to be "Off" with capital? Don't you need to do a case insensitive
compare?
Line 268: }
Line 269:
Line 270:
Line 271: public final class FileFetchEventArgs extends EventArgs {
--
To view, visit http://gerrit.ovirt.org/16224
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied777eaefe6c6dc25a002e761bbf5b501710b2ae
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
