Vojtech Szocs has posted comments on this change. Change subject: gluster-nagios-monitoring : messageOrigin url formation fixed ......................................................................
Patch Set 2: (2 comments) http://gerrit.ovirt.org/#/c/27396/2/gluster-nagios-monitoring/src/js/dashboard-init.js File gluster-nagios-monitoring/src/js/dashboard-init.js: Line 61: mod.factory('initService', ['pluginApi', 'pluginEventHandlers', '$window', '$location', function (pluginApi, pluginEventHandlers, $window, $location) { Line 62: return { Line 63: bootstrapPlugin: function () { Line 64: var messageOrigin; Line 65: if(!pluginApi.configObject().messageOrigins || !pluginApi.configObject()){ First of all, pluginApi.configObject() will always be defined, there's no need to check that. Runtime UI plugin configuration consists of custom "config" (i.e. myplugin-config.json) object, if available, merged on top of default "config" (i.e. myplugin.json) object, if available. If there is no custom and default "config" defined, configObject method returns an empty object {}. Second, I suggest to modify "if" condition to make it more readable: if (pluginApi.configObject().messageOrigins) { messageOrigin = pluginApi.configObject().messageOrigins; } else { // messageOrigins not defined in runtime plugin config messageOrigin = $location.protocol() ... } Or even better: var messageOrigin = pluginApi.configObject().messageOrigins; if (!messageOrigin) { // messageOrigins not defined in runtime plugin config messageOrigin = $location.protocol() ... } Please use "if (" (with space) and not "if(" (without space) because the latter appears like function invocation at first sight. Finally, ternary operator (testExpression ? truthyExpression : falsyExpression) is best used separately, as it can make code less readable. For example: // using ternary operator separately to improve readability var port = ( ... ) ? ":" + $location.port() : ""); messageOrigin = $location.protocol() + "://" + $location.host() + port; Line 66: messageOrigin = $location.protocol() + "://" + $location.host() + ((($location.port()) && ($location.absUrl().indexOf($location.port().toString()) > 0)) ? ":" + $location.port() : ""); Line 67: } else { Line 68: messageOrigin = pluginApi.configObject().messageOrigins; Line 69: } http://gerrit.ovirt.org/#/c/27396/2/gluster-nagios-monitoring/src/js/trends.js File gluster-nagios-monitoring/src/js/trends.js: Line 163: * the pnp4nagios server. Line 164: */ Line 165: var pnp4NagiosUrl = ""; Line 166: if((!configObject.pnp4nagiosUrl) || (!configObject)) { Line 167: pnp4NagiosUrl = $location.protocol() + "://" + $location.host() + (($location.port()) ? ":" + $location.port() : ""); Please see my comment in dashboard-init.js about the ternary operator, it's up to your consideration. Line 168: } else { Line 169: pnp4NagiosUrl = configObject.pnp4nagiosUrl; Line 170: } Line 171: return pnp4NagiosUrl; -- To view, visit http://gerrit.ovirt.org/27396 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2dd11b3accc83fe4a603a930c4fcaf9c3261a271 Gerrit-PatchSet: 2 Gerrit-Project: samples-uiplugins Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: anmolbabu <[email protected]> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
