Re: [JGIT] Request for help

Thanks Jonas!

I now squashed a lot of commits together where possible and republished the rebased branch. 

Next steps:

* mavenizing org.spearce.jgit.pgm

LieGrue,
strub

--- On Fri, 9/4/09, Jonas Fonseca <jonas.fonseca@gmail.com> wrote:

From: Jonas Fonseca <redacted>
Subject: Re: [JGIT] Request for help
To: "Mark Struberg" <redacted>
Cc: "Douglas Campos" <redacted>, git@vger.kernel.org, "Gabe McArthur" <redacted>
Date: Friday, September 4, 2009, 8:50 PM
On Fri, Sep 4, 2009 at 13:28, Mark
Struberg[off-list ref]
wrote:

quoted

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
--
To unsubscribe from this list: send the line "unsubscribe
git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html