Kanagaraj M has posted comments on this change.

Change subject: webadmin : Change volume status to yellow when bricks down
......................................................................


Patch Set 6:

(2 comments)

....................................................
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/VolumeStatusCell.java
Line 44:                 statusImage = resources.downImage();
Line 45:                 tooltip = constants.down();
Line 46:                 break;
Line 47:             case UP:
Line 48:                 if(isBrickDown(volume) == brickStatus.allBricksDown || 
isBrickDown(volume) == brickStatus.someBricksDown) {
here the no.of.bricks down is calculated twice if not all the bricks are down
Line 49:                     statusImage = resources.volumeUpWarning();
Line 50:                     tooltip = constants.volumeBricksDown();
Line 51:                 } else {
Line 52:                     statusImage = resources.upImage();


Line 68:             sb.append(alertImageHtml);
Line 69:         }
Line 70:     }
Line 71: 
Line 72:     public brickStatus isBrickDown(GlusterVolumeEntity volume) {
Agree.

But my question here is why not return the count from this method and in the 
above method compare it with no.of.bricks.

And if the method name starts with "is", it supposed to return a boolean
Line 73:         int count = 0;
Line 74:         for (GlusterBrickEntity brick : volume.getBricks()) {
Line 75:             if (brick.getStatus() == GlusterStatus.DOWN) {
Line 76:                 count++;


-- 
To view, visit http://gerrit.ovirt.org/19611
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iddaf1676bf9bf2c3aa0d4381e5057e21d4c8512e
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <anb...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: anmolbabu <anb...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to