[email protected] commented on branch /branches/2.0-maven in project google-guice. Details are at http://code.google.com/p/google-guice/source/branch?spec=issue393&branch=%2Fbranches%2F2.0-maven
Score: Positive Line-by-line comments: File: /branches/2.0-maven/extensions/pom.xml (r1016) =============================================================================== Line 11: <artifactId>extension</artifactId> ------------------------------------------------------------------------------- This should probably be extension-parent, or guice-extension-parent as was suggested by other commenters. File: /branches/2.0-maven/guice-parent/pom.xml (r1016) =============================================================================== Line 54: <url>http://code.google.com/p/google-guice/source/browse/</url> ------------------------------------------------------------------------------- Could we also add connection and developerConnection here? That will allow m2eclipse users (at least) to check out the code. <connection>scm:svn: http://google-guice.googlecode.com/svn/trunk/</connection> <developerConnection>scm:svn:https://google-guice.googlecode.com/svn/trunk/ </developerConnection> Line 57: <plugins> ------------------------------------------------------------------------------- This should probably go into a pluginManagement section instead of a plugins sections. Otherwise it gets attached to the build lifecycle even in pom-packaged projects. I know we're not using maven to build with. It's just a matter of tidiness. :) File: /branches/2.0-maven/spring/pom.xml (r1016) =============================================================================== Line 23: <version>2.0.2</version> ------------------------------------------------------------------------------- That's quite an old version of Spring. Is that just because that's what guice currently uses? Looking in lib/build/spring-beans!META-INF/MANIFEST.MF it appears to be 2.0.6. Either 2.0.8 or 2.5.6 is the latest on central. Line 24: <scope>provided</scope> ------------------------------------------------------------------------------- Is there a reason for making this provided? Surely if you want to depend on guice-spring, you'll want to pull in spring transitively too? File: /branches/2.0-maven/struts2/plugin/pom.xml (r1016) =============================================================================== Line 28: <scope>provided</scope> ------------------------------------------------------------------------------- I'm not sure why this needs to be provided scope. Line 34: <scope>provided</scope> ------------------------------------------------------------------------------- Same here — why provided? Respond to these comments at http://code.google.com/p/google-guice/source/branch?spec=issue393&branch=%2Fbranches%2F2.0-maven -- You received this message because you starred this review, or because your project has directed all notifications to a mailing list that you subscribe to. You may adjust your review notification preferences at: http://code.google.com/hosting/settings --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "google-guice-dev" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en -~----------~----~----~----~------~----~------~--~---
