Benjamin Thanks for a very detailed review, I will update the source base on your comments.
Thanks a bunch -Dan On Sat, Dec 6, 2008 at 2:51 PM, Benjamin Bentmann <[EMAIL PROTECTED]> wrote: > Dan Tran wrote: > >> Please vote. > > 0, not using it. Notes: > > "mvn dependency:analyze" reports some undeclared ones. > > "mvn docck:check" reports a missing <prerequisite> element. Seems to be > 2.0.8 from the deps but currently defaults to 2.0. > > The site.xml is missing the "Goals" (i.e. plugin-info.html) link. > > There's a typo in the project <description>: > "[...] Supports Provides [...]" > > Maybe the project's SCM <url> should point to FishEye's web view. > > The links to "wagon:copy" and "wagon:merge-maven-repos" on the index page > point into nowhere (missing "-mojo" suffix in the target file name). > > When you release the plugin, be sure to remember to update the POM snippets > on the usage page to give the release version instead of the SNAPSHOT. > > In the UploadMojo, should it really read "userDefaultExcludes" instead of > "useDefaultExcludes" (note the missing "r")? > > Another cosmetic thing: > boolean skip (not isSkip) > but > boolean isCaseSensitive > Wouldn't > boolean caseSensitive > be more consistent and ease usage? > > A little more irritating: The parameter isCaseSensitive does not have the > default-value annotation (in AbstractWagonListMojo and CopyMojo), hence the > default value is not explicitly mentioned on the site. Java developers might > assume it to be false just as for boolean instance variables, but it's > actually true in the plugin. > > If I see properly, wagon:copy uploads files to the same relative directory > on the target wagon as given by fromDir for the source wagon. I was > originally wondering why there's no "toDir" parameter, should that be > documented for clarity? > > The DefaultWagonUpload errors out on optimize=true when the repo URL does > not start with "scp://". I haven't actually tested it, but wouldn't > "scpexe:" and "sftp:" support the optimized upload as well? Would it make > more sense to just check that the used wagon is an instance of > CommandExecutor given that's that the thing the code actually expects/casts? > > > Benjamin > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > > --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email
