Re: [JGIT] Request for help

> From: Jonas Fonseca <jonas.fonseca@gmail.com>
> Subject: Re: [JGIT] Request for help
> To: "Mark Struberg" <struberg@yahoo.de>
> Cc: "Douglas Campos" <douglas@theros.info>, git@vger.kernel.org, "Gabe McArthur" <gabriel.mcarthur@gmail.com>
> Date: Friday, September 4, 2009, 8:50 PM
> On Fri, Sep 4, 2009 at 13:28, Mark
> Struberg<struberg@yahoo.de>
> wrote:
> > Hi!
> >
> > Work has been done at
> >
> > http://github.com/sonatype/JGit/tree/mavenize
> >
> > Please feel free to pull/fork and share your changes!
> I'd be happy to pull it in.
> 
> IMO, there are a lot of things that can be squashed
> together and
> cleaned up. I know that you advocated for incremental
> introduction,
> but it seems wrong to for example add a file and then
> completely
> reformat it a few commits later. The same thing with the
> .gitignore
> fixes in step 5.
> 
> Some comments ... Some of them I initially entered in
> github's
> codereview, but I ended up writing it all here.
> 
> Commit: "mavenizing step 1: moved over the initial poms
> from Jasons branch"
> 
>  * Please always add an empty line between the subject and
> the body
>    of the commit message. Like this:
> 
>   mavenizing step 1: moved over the initial poms from
> Jasons branch
> 
>   Signed-off-by: Mark Struberg >struberg@yahoo.de>
> 
>  * The .gitignore pattern could be further limited to
> "target/" ...
> but you seem to change this to /target later.
> 
> In org.spearce.jgit/pom.xml:
> 
>     * The use of maven-surefire-plugin should be
> removed. This module
> does not have any tests.
> 
>     * Shouldn't we retain the original
> ${groupId}:${artifactId} naming
> convention, being org.spearce:jgit?
> 
> In org.spearce.jgit.test/pom.xml:
> 
>     * Dependency on jsch is unecessary since it
> is derived from
> org.spearce.jgit.
> 
>     * Maybe name as org.spearce:jgit-test?
> 
> In org.spearce.jgit.pgm/pom.xml:
> 
>     * Maybe name as org.spearce:jgit-pgm?
> 
> Commit: "mavenizing step 2: move the core libs from src to
> src/main/java"
> 
>  * Please also add an empty line to this commit message.
> 
>  * You might as well squash the whitespace fixes into the
> first commit.
> 
> Commit: "mavenizing step 3: moving all core tests into the
> core module"
> 
>  * The commit message wrongly states:
>     org.spearce.jgit.test/tst/ ->
> org.spearce.jgit/src/test/java/tst/
>    Should be:
>     org.spearce.jgit.test/tst/ ->
> org.spearce.jgit/src/test/java/
> 
> Commit: "mavenizing step 4: moving some license files and
> META-INF"
> 
>  * Shouldn't the commit message rather say "remove JSch"?
>    Then the moving of META-INF can be put in
> its own commit.
> 
>  * The new NOTICE file has a few typos and the info could
> fit into the README
> 
> Then I got a bit lost in a huge reformatting.
> 
> -- 
> Jonas Fonseca
>