Vojtech Szocs has posted comments on this change.

Change subject: webadmin: DC subtab refreshes shows loading indicator
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/25593/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java:

Line 82:         if (value == null) {
Line 83:             this.comment = "";
Line 84:         } else {
Line 85:             this.comment = value;
Line 86:         }
> Checkstyle discourages the use of the ternary operator. I agree with that. 
For very simple cases like this, I'm also leaning towards ternary operator, but 
I understand that in general, ternary operator can make things less readable.

Not sure if we should do the assert here, because AFAIK if we run JBoss with 
assertions enabled and assert will fail, it will throw AssertionError. In this 
case, comment value being null isn't something (I think) that should throw 
AssertionError (along with consequences such as breaking current flow & logging 
the error).

(In GWT world, assert has a whole different meaning, though - it only prints 
assert errors in GWT Dev Mode console, in GWT production mode asserts are 
compiled-out. So in GWT world, assert would make perfect sense here.)
Line 87:     }
Line 88: 
Line 89:     @Override
Line 90:     public Guid getId() {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ae44a6ea127b4e3b35647721fd2e927bac130b6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to