Einav Cohen has posted comments on this change.

Change subject: webadmin: 'remove' confirmation dialog message cleanup
......................................................................


Patch Set 1:

(2 comments)

....................................................
Commit Message
Line 7: webadmin: 'remove' confirmation dialog message cleanup
Line 8: 
Line 9: Changed the 'remove' confirmation dialog to use a default message, but 
allow
Line 10: an override. Cleaned up useless calls to set dialog's message to 
prevent
Line 11: regressions. Removed related unused UI constants.
I suggest to amend the commit comment to include a little bit more background 
and explanations, and also - need to separate to paragraphs (for readability).
i.e.:

"""

before this patch, 'remove' confirmation dialog message contained a default 
message that could have been overridden only if the dialog's HashName didn't 
start with "remove_".

in this patch:

- 'remove' confirmation dialog still contains a default message, but now it can 
always be overridden, regardless of the dialog's hashName value.

- Cleaned up (now unnecessary) HashName manipulation in RemoveNetworkQoSModel.

- Cleaned up useless calls to set dialog's message (which were probably useful 
up until http://gerrit.ovirt.org/#/c/14621/ has been merged) in order to 
prevent regressions such as displaying "Virtual Machines" as the dialog's 
content instead of the default "Are you sure you want to remove the following 
items?" message.

- Removed the related (now unused) UI constants.

"""

Thanks.
Line 12: 
Line 13: Change-Id: I7411dec0eaf8d60cefff563988618f7c548eecce
Line 14: Bug-Url: https://bugzilla.redhat.com/1021584


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/RemoveNetworkQoSModel.java
Line 67
Line 68
Line 69
Line 70
Line 71
I see that we actually had some places in which the HashName was manipulated 
just so the dialog message could be overridden... I am glad that we are fixing 
the code so this kind of manipulations wouldn't be necessary anymore.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7411dec0eaf8d60cefff563988618f7c548eecce
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[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