Juan Hernandez has posted comments on this change.

Change subject: core: Make dependency on commons-collections explicit
......................................................................


Patch Set 1:

> 1. If your code imports something - a dependency should be added to the 
> pom.xml of the project

Yes, I think that is a good practice. Things may work if you don't do it, 
because Maven may bring the dependency automatically for you due to 
transitivity, but it is bad idea to count on that.

> 2. If that import depends on something else - don't add it to the pom (e.g., 
> if I added commons-logging, I should not explicitly add log4j, maven's 
> dependency mechanism should handle that

Yes, correct, although I have seen situations where this doesn't work 
correctly, usually due to bugs in Maven or in the POMs of the artifacts that 
your are using. For example, if the POM of commons-logging is wrong and doesn't 
specify the dependency on log4j you may need to make the dependency on log4j 
explicit in your own POM, but this is not that usual.

> 3. versions should be defined in the master pom, not in each project.

Yes, in the dependencyManagement section, that way when you need to add a 
dependency for an artifact in some sub-project you only need to specify the 
artifact group and artifact id, not the version number. This helps to make sure 
that you are using the same version of the artifact all over your project.

This is my view, I could be wrong, of course :-)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id46b4d7a360348a67fa7430b4af0eefa3d76fe2a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to