Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: UI: libosinfo - validate VM minimum RAM with libosinfo
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(5 inline comments)

Relative minor comments inline.

....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java
Line 1085:     @DefaultStringValue("CPU Architecture")
Line 1086:     String cpuArchVmGeneral();
Line 1087: 
Line 1088:     @DefaultStringValue("CPU Architecture")
Line 1089:     String cpuArchTemplateGenral();
Shouldn't this be General instead of Genral?
Line 1090: 
Line 1091:     @DefaultStringValue("CPU Architecture")
Line 1092:     String cpuArchPoolGeneral();


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/template/TemplateGeneralModelForm.java
Line 49:         formBuilder.addFormItem(new 
FormItem(constants.nameTemplateGeneral(), name, 0, 0));
Line 50:         formBuilder.addFormItem(new 
FormItem(constants.descriptionTemplateGeneral(), description, 1, 0));
Line 51:         formBuilder.addFormItem(new 
FormItem(constants.hostClusterTemplateGeneral(), hostCluster, 2, 0));
Line 52:         formBuilder.addFormItem(new 
FormItem(constants.osTemplateGeneral(), oS, 3, 0));
Line 53:         formBuilder.addFormItem(new 
FormItem(constants.cpuArchTemplateGenral(), cpuArch, 4, 0));
Same about genral vs general.
Line 54:         formBuilder.addFormItem(new 
FormItem(constants.defaultDisTypeTemplateGeneral(), defaultDisplayType, 5, 0));
Line 55: 
Line 56:         formBuilder.addFormItem(new 
FormItem(constants.definedMemTemplateGeneral(), definedMemory, 0, 1));
Line 57:         formBuilder.addFormItem(new 
FormItem(constants.numOfCpuCoresTemplateGeneral(), cpuInfo, 1, 1));


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
Line 308:               <include name="common/action/gluster/*.java"/>
Line 309:               <include name="common/queries/gluster/*.java"/>
Line 310:               <include 
name="common/constants/gluster/GlusterConstants.java"/>
Line 311:               <include 
name="common/utils/gluster/GlusterCoreUtil.java"/>
Line 312: 
I see no reason to add a newline here.
Line 313:       </source>
Line 314: 
Line 315:       <super-source path="ui/uioverrides" />


....................................................
File frontend/webadmin/modules/uicommonweb/pom.xml
Line 43:                        <artifactId>libosinfo-types</artifactId>
Line 44:                        <version>${engine.version}</version>
Line 45:                </dependency>
Line 46: 
Line 47:        </dependencies>
Please indent with a tab here.
Line 48: 
Line 49:        <build>
Line 50:                <plugins>
Line 51:                        <plugin>


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
Line 2563:             }
Line 2564:             // fill arch type list by finding resources with arch of 
x84_64
Line 2565:             for (Resources resources : os.getResources()) {
Line 2566:                 if 
(CpuArch.X86_64.name().equalsIgnoreCase(resources.getArch()))
Line 2567:                 {
Just wondering: is there a convention for braces in engine?
Line 2568:                     
x64OsTypes.add(VmOsType.getByShortId(os.getShortId()));
Line 2569:                 }
Line 2570:             }
Line 2571:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie96ede540a069c29db95a899551f228309f3a17f
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ew...@kohlvanwijngaarden.nl>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to