Vojtech Szocs has posted comments on this change.

Change subject: userportal,webadmin: Update GeneralFormPanel
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

Nice patch!

https://gerrit.ovirt.org/#/c/37527/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormBuilder.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/FormBuilder.java:

Line 34:      * @param widthInColumns The number of grid columns you want your 
column to occupy.
Line 35:      */
Line 36:     public void setRelativeColumnWidth(int columnNum, int 
widthInGridColumns) {
Line 37:         if (widthInGridColumns < 1 || widthInGridColumns > 12) {
Line 38:             throw new IllegalArgumentException("The widthInGridColumns 
has to be between 1 and 12"); //$NON-NLS-1$
An alternative to throwing exception (that does break current control flow, 
possibly leaving UI in inconsistent state) is to use Java "assert" (that gets 
pruned out of compiled JavaScript, so the current control flow isn't broken).

Anyway, I'm OK with throwing an exception here.
Line 39:         }
Line 40:         formPanel.setRelativeColumnWidth(columnNum, 
widthInGridColumns);
Line 41:     }
Line 42: 


-- 
To view, visit https://gerrit.ovirt.org/37527
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f50ce04fa064a22d3f124360918c31db599e105
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[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