Robert, On 16 September 2010 21:38, Robert Scholte <[email protected]> wrote: > Mark, > > right now I see a bit too much issues to give it a +1. > > here are a few of my remarks, purely based on code and sitedocs > - check http://mojo.codehaus.org/apt-maven-plugin/checkstyle.html : 300+ > errors is really too much. With the right IDE-settings they can be fixed in > seconds. Read http://maven.apache.org/developers/conventions/code.html for > the right templates.
The 324 WhitespaceAround errors are generics syntax errors since mojo-parent uses maven-checkstyle-plugin:2.2 for reports, which in turn uses Checkstyle 4.x, which does not support Java5 syntax. If I upgrade this to 2.5 then we have 14 errors, 10 of which are due to using a MIT license rather than the ASL one. I'll commit this to mojo-parent. > - Both ProcessMojo and TestProcessMojo have includes/excludes parameters and > related getter-method. I can't see any reason why not to pull them up to > AbstractAptMojo Sure, can be done for the next release. > - Expressions and default-values are often mixed up. > - Both parameters of EclipseMojo should use default-value instead of > expression > - For ProcessMojo: compileSourceRoots, resources, classpathElements > - For TestProcessMojo: testCompileSourceRoots, testResources, > testClasspathElements Expressions are used for read-only parameters, since a non-default value of a read-only parameter makes no sense. Default values are used for writeable parameters. > - FAQ has a reference to http://jira.codehaus.org/browse/MCOMPILER-75 . This > issue is closed as 'fixed', which could suggest that this might be one of > the final heartbeats of this plugin. What's your idea on this one? maven-compiler-plugin is the way to go in future but it can only be used with Java6+. apt-maven-plugin supports Java5. It is also missing a couple of features: being able to specify processor dependencies within the plugin dependencies rather than the exported project dependencies; and staleness checking [1]. It is also being used by several projects and so cannot be retired yet. [1] http://mojo.codehaus.org/apt-maven-plugin/examples/configuring-staleness-checking.html > - mvn dependency:analyze shows me the following: > [WARNING] Used undeclared dependencies found: > [WARNING] org.apache.maven:maven-model:jar:2.0.2:compile > [WARNING] org.apache.maven:maven-artifact:jar:2.0.2:compile > [WARNING] > org.codehaus.plexus:plexus-container-default:jar:1.0-alpha-9:compile These are transitive dependencies of maven-project. I don't believe that it's necessary to declare used dependencies that are expected to be transitive from higher-level dependencies. I'd rather not go back to those Maven 1.x days.. > - Maven prerequisite value doesn't match the version of the used maven > artifacts. I'd suggest to add <mavenVersion>2.0.2</mavenVersion> as property > (or 2.0.6) and use this property for both the dependencies and the > prerequisite. The minimum version of Maven required to build and run the plugin is set to 2.0.6 since we use plexus-utils > 1.1 [2]. The actual minimum version of maven-project required to compile is 2.0.2, but is in fact whatever version of Maven that is being used at runtime [3], i.e. >= 2.0.6. So we can certainly align these and upgrade maven-project to 2.0.6. [2] http://maven.apache.org/plugin-developers/common-bugs.html#Depending_on_Plexus_Utilities_1.1 [3] http://maven.apache.org/guides/mini/guide-maven-classloading.html > btw, it seems like you managed to bypass MSITE-459. It's a pretty deep > transitive dependency which causes the trouble, but it looks like it's > solved in the core (still SNAPSHOT). It can take a while before this solved > in the m-site-plugin as well, so I'd suggest you add this to the mojo-parent > as well. Looks like the real fix will be upgrading wagon-webdav to 1.0-beta-7 in mojo-parent, see WAGON-60. Not sure when this is planned to be released, but there's only two issues left that are targeted for this version. So I can either add my maven-site-plugin workaround to mojo-parent or we can wait for wagon 1.0-beta-7 to be released? These are all minor issues which can easily be addressed in the next version, especially as this is an alpha release. If you're not +1 are you vetoing this release? Mark --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email
