Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: core: WIP - libosinfo - introduce libosinfo service
......................................................................


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

(11 inline comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmOsType.java
Line 67:     }
Line 68: 
Line 69:     public String getShortId() {
Line 70:         return shortId;
Line 71: 
Mind removing this empty line?
Line 72:     }


....................................................
File 
backend/manager/modules/services/libosinfo/interface/src/main/java/org/ovirt/engine/core/services/libosinfo/LibosinfoServiceXmlImpl.java
Line 28:     private Map<String, Os> osInfoMap = new HashMap<String, Os>();
Line 29: 
Line 30:     private final ObjectFactory factory = new ObjectFactory();
Line 31:     private final Os emptyOs = factory.createOs();
Line 32:     private final Os.Resources emptyResource = 
factory.createOsResources();
I think you can make these empty items static.
Line 33: 
Line 34:     private LibosinfoServiceXmlImpl() {
Line 35:         init();
Line 36:     }


....................................................
File 
backend/manager/modules/services/libosinfo/interface/src/test/java/org/ovirt/engine/core/services/libosinfo/LibosinfoServiceTest.java
Line 26:         }
Line 27:     }
Line 28: 
Line 29:     @Test
Line 30:     public void testRemmendedCpu() {
I think you mean testRecommendedCpu here.
Line 31:         
Assert.assertTrue(service.getRecommendedCpu(VmOsType.Windows2003.name(), 
CpuArch.i386) > 100);
Line 32:     }
Line 33: 
Line 34:     @Test


Line 31:         
Assert.assertTrue(service.getRecommendedCpu(VmOsType.Windows2003.name(), 
CpuArch.i386) > 100);
Line 32:     }
Line 33: 
Line 34:     @Test
Line 35:     public void testRemmendedRam() {
testRecommendedRam?
Line 36:         
Assert.assertTrue(service.getRecommendedRam(VmOsType.Windows2008.name(), 
CpuArch.i386) > 100);
Line 37:     }
Line 38: 
Line 39:     @Test


....................................................
File backend/manager/modules/services/libosinfo/types/pom.xml
Line 27:           <generatePackage>${groupId}</generatePackage>
Line 28:          <extension>true</extension>
Line 29:           <args>
Line 30:   <!--      
Line 31:            GWT isn't familiar with the strategy classes that jvnet 
supply for 
Trailing whitespace.
Line 32:            the equals,toString and so on so ignoring this 
functionality for now.
Line 33: 
Line 34:            <arg>-XtoString</arg>
Line 35:             <arg>-Xequals</arg>


Line 34:            <arg>-XtoString</arg>
Line 35:             <arg>-Xequals</arg>
Line 36:             <arg>-XhashCode</arg>
Line 37:             <arg>-Xcopyable</arg>
Line 38:            -->
Mixing tabs and spaces here.
Line 39:           </args>
Line 40:           <plugins>
Line 41:             <plugin>
Line 42:               <groupId>org.jvnet.jaxb2_commons</groupId>


Line 56:              <goal>jar-no-fork</goal>
Line 57:                </goals>
Line 58:               </execution>
Line 59:              </executions>
Line 60:            </plugin>
Mixing tabs and spaces.
Line 61: 
Line 62:     </plugins>
Line 63:   </build>
Line 64:   <dependencies>


....................................................
File backend/manager/modules/services/libosinfo/types/src/main/resources/os.xsd
Line 14:               <xs:element type="xs:string" name="name"/>
Line 15:               <xs:element type="xs:float" name="version" 
minOccurs="0"/>
Line 16:               <xs:element type="xs:string" name="vendor"/>
Line 17:               <xs:element type="xs:string" name="family"/>
Line 18:               <xs:element name="devices" type="Devices"/>       
Trailing whitespace.
Line 19:               <xs:element name="derives-from" minOccurs="0">
Line 20:                 <xs:complexType>
Line 21:                   <xs:simpleContent>
Line 22:                     <xs:extension base="xs:string">


Line 89:                         </xs:simpleContent>
Line 90:                       </xs:complexType>
Line 91:                     </xs:element>
Line 92:                   </xs:sequence>
Line 93:                 </xs:complexType>
Somewhat inconsistent indenting?


....................................................
File frontend/webadmin/modules/uicompat/pom.xml
Line 38:       <artifactId>libosinfo-types</artifactId>
Line 39:       <classifier>sources</classifier>
Line 40:       <version>${project.version}</version>
Line 41:     </dependency>
Line 42:    <dependency>
Inconsistent indenting.
Line 43:       <groupId>com.google.gwt</groupId>
Line 44:       <artifactId>gwt-user</artifactId>
Line 45:       <version>${gwt.version}</version>
Line 46:     </dependency>


Line 43:       <groupId>com.google.gwt</groupId>
Line 44:       <artifactId>gwt-user</artifactId>
Line 45:       <version>${gwt.version}</version>
Line 46:     </dependency>
Line 47: </dependencies>
No need to remove the indenting here.
Line 48:   <build>
Line 49:     <resources>
Line 50:       <resource>
Line 51:         <directory>src/main/java</directory>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I663dbae36d583ca4c524b2957a9a695e44207975
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ew...@kohlvanwijngaarden.nl>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to