Re: [JGIT] Request for help

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> wrote:

> From: Jonas Fonseca <jonas.fonseca@xxxxxxxxx>
> Subject: Re: [JGIT] Request for help
> To: "Mark Struberg" <struberg@xxxxxxxx>
> Cc: "Douglas Campos" <douglas@xxxxxxxxxxx>, git@xxxxxxxxxxxxxxx, "Gabe McArthur" <gabriel.mcarthur@xxxxxxxxx>
> Date: Friday, September 4, 2009, 8:50 PM
> On Fri, Sep 4, 2009 at 13:28, Mark
> Struberg<struberg@xxxxxxxx>
> 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@xxxxxxxx>
> 
>  * 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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


      
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]