Vojtech Szocs has posted comments on this change.
Change subject: Shell-In-A-Box UI plugin - adding RPM spec file
......................................................................
Patch Set 4: (4 inline comments)
Just some minor comments, nothing serious :-)
....................................................
File shellinabox-plugin/README
Line 4:
Line 5: < Engine >
Line 6: * Create tarball file:
Line 7: - git clone git://gerrit.ovirt.org/samples-uiplugins
Line 8: - cd samples-uiplugins/shellbox-plugin
"cd samples-uiplugins/shellinabox-plugin" ?
Line 9: - git archive --prefix=ovirt-engine-shellinabox-uiplugin-0.1/
--output=ovirt-engine-shellinabox-uiplugin-0.1.tar HEAD .
Line 10: * Build and install RPM:
Line 11: - Copy 'ovirt-engine-shellinabox-uiplugin-0.1.tar' to SOURCES folder
Line 12: - Build RPM using 'ovirt-engine-shellinabox-uiplugin.spec' file
....................................................
File shellinabox-plugin/shellinabox-files/start.html
Line 11:
Line 12: // Add 'Shell In A Box' button (+ context menu) to 'Hosts'
main-tab
Line 13: api.addMainTabActionButton('Host', 'Shell In A Box', {
Line 14: onClick : function() {
Line 15: window.open(getShellInABoxUrl(arguments),
'_blank');
Since "isEnabled == true" only when a single host is selected, you can pass
"arguments[0]" instead of "arguments" to getShellInABoxUrl function.
Line 16: },
Line 17: isEnabled : function() {
Line 18: // The button is enabled only when a single host
is selected
Line 19: return arguments.length == 1;
Line 22: },
Line 23: HostSelectionChange : function() {
Line 24: if (arguments.length == 1) {
Line 25: // Update iframe URL on host selection
Line 26: api.setTabContentUrl('shellinabox',
getShellInABoxUrl(arguments));
Considering my comment above, you can pass "arguments[0]" instead of
"arguments" to getShellInABoxUrl function.
Line 27: }
Line 28: }
Line 29: });
Line 30: api.ready();
Line 29: });
Line 30: api.ready();
Line 31:
Line 32: // Get 'Shell In A Box' URL using specified host address
Line 33: var getShellInABoxUrl = function(arguments) {
Considering my comment above, you can change the signature of this function
like this:
var getShellInABoxUrl = function(selectedHost) {
var hostAddress = selectedHost.name;
...
}
Line 34: var hostAddress = arguments[0].name;
Line 35: var port = '4200';
Line 36: var shellUrl = 'http://' + hostAddress + ':' + port;
Line 37:
--
To view, visit http://gerrit.ovirt.org/11198
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If916c7cf022e9e903cd06e55f7b8ff34851ab9c8
Gerrit-PatchSet: 4
Gerrit-Project: samples-uiplugins
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
