Juan Hernandez has posted comments on this change.

Change subject: core: introduce osinfo service implementation over RMI
......................................................................


Patch Set 2: (3 inline comments)

I think it may be worth to reduce the number of classes generated with 
Jnaearator, maybe eliminate them completely. I think that what need can be 
written manually, as we don't use most of the generated code.

....................................................
File backend/manager/tools/pom.xml
Line 60:       <scope>test</scope>
Line 61:     </dependency>
Line 62: 
Line 63:     <dependency>
Line 64:       <groupId>net.java.dev.jna</groupId>
I see "com.sun" as group id in some places and "net.java.dev.jna" in other 
places. What is the difference?
Line 65:       <artifactId>jna</artifactId>
Line 66:       <version>${jna.version}</version>
Line 67:     </dependency>
Line 68: 


....................................................
File 
backend/manager/tools/src/main/modules/org/ovirt/engine/core/tools/main/module.xml
Line 15:       <module name="org.apache.log4j"/>
Line 16:       <module name="org.ovirt.engine.core.common"/>
Line 17:       <module name="org.ovirt.engine.core.compat"/>
Line 18:       <module name="org.ovirt.engine.core.dependencies"/>
Line 19:       <module name="org.ovirt.engine.core.utils" export="true"/>
Why is this needed? If possible explicitly add the dependency to the module 
that needs it instead of exporting it here.
Line 20:       <module name="org.postgresql"/>
Line 21:       <module name="sun.jdk"/>
Line 22:       <module name="com.sun.jna"/>
Line 23:    </dependencies>


....................................................
File pom.xml
Line 82:     <commons-jxpath.version>1.3</commons-jxpath.version>
Line 83:     <jaxb-impl.version>2.2</jaxb-impl.version>
Line 84:     <jbosssx-bare.version>2.0.4</jbosssx-bare.version>
Line 85:     <log4j.version>1.2.16</log4j.version>
Line 86:     <jna.version>3.4.0</jna.version>
I would suggest to add the depencency in the "depencencyManagement" section 
below, that way you don't have to repeat the version number in every POM you 
want to use JNA.
Line 87: 
Line 88:     <!-- Plugin Versions -->
Line 89:     
<maven-compiler-plugin.version>2.3.2</maven-compiler-plugin.version>
Line 90:     <gwt.plugin.version>2.3.0</gwt.plugin.version>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia893bdc3dfc486e0b3e735f4e187996710a05f54
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to