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