Shubhendu Tripathi has posted comments on this change.

Change subject: gluster: Add support for gluster brick creation
......................................................................


Patch Set 15:

(11 comments)

https://gerrit.ovirt.org/#/c/36031/15//COMMIT_MSG
Commit Message:

Line 12:  
%s/cab/can be/g


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
File 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java:

Line 3202: Cannot
Ideally the action and entity type is place holder and should be used that way.


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/help/HelpTag.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/help/HelpTag.java:

Line 566:     remove_cpu_profile("remove_cpu_profile", HelpTagType.WEBADMIN, 
"Clusters Tab > Cpu Profiles sub-tab -> Remove Cpu Profile dialog"), 
//$NON-NLS-1$ //$NON-NLS-2$
Line 567: 
Line 568:     numa_support("numa_support", HelpTagType.WEBADMIN, "Host Tab > 
NUMA Support"); //$NON-NLS-1$ //$NON-NLS-2$
Line 569: 
Line 570: 
remove the unwanted line
Line 571:     public final String name;
Line 572: 
Line 573:     HelpTag(String name, HelpTagType type) {
Line 574:         this.name = name;


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/HostGlusterStorageDevicesListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/HostGlusterStorageDevicesListModel.java:

Line 176: OnCreateBrick
camelCase for command names


Line 181: createDefaultCancelUiCommand
same as above


Line 247: OnCreateBrick
command names with camelCase ??


Line 249: equals
same as above


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java:

Line 4120: Choose
Do we need the "(" in the start here ??


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/gin/uicommon/HostModule.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/gin/uicommon/HostModule.java:

Line 329: 
Not required ??


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/CreateBrickPopupView.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/CreateBrickPopupView.java:

Line 121: addColumn
I remember AbstractTextColumnWithTooltip does not exist now. please verify


https://gerrit.ovirt.org/#/c/36031/15/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
File 
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties:

Line 1151: ACTION_TYPE_FAILED_STORAGE_DEVICE_LOCKED
Use place holder for action and entity type


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If43b6503dd8362d2a0907ac9648335a750828427
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ramesh N <rnach...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Ramesh N <rnach...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com>
Gerrit-Reviewer: anmolbabu <anb...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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