Re: Code review tool? (and [PATCH] fix for stencil rotating)

[Reinhold Kainhofer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am Mittwoch, 10. September 2008 schrieb Han-Wen Nienhuys:
> I agree that it's not perfect, but from
> my perspective as reviewer, it's actually much better than using mail
> directly.  

I agree to that. The ability to add comments directly into the patch (and 
reply to them, although that functionality currently seems broken, as I 
always get a failed assertion when I try to reply to a comment) is really 
useful.

> The per-file rather than per patch-set comment confuses me, as SVN
> uses changesets too. Do you mean that you want a view that includes
> all diffs in a single page?

Actually, what I meant was that I don't seen an easy way to work with multiple 
atomic patches, that depend on each other (i.e. upload a whole local branch 
for review, when all the features have been implemented). It appears that 
git-cl will not honor these patch sets, but rather create one huge diff of 
all changed files rather than a set of patches. This also means that all 
explanations in the git commit messages will be lost/ignored and you'll have 
to add the explanation manually.

In svn you can only have uncommitted changes, once you commit, everything will 
be on the server, so this approach makes sense. But in git, you usually first 
commit (locally) several times before you send / publish a patch for review, 
so you have lots of local commits, which should normally not be merged to one 
big commit.

> On Wed, Sep 10, 2008 at 6:48 AM, Reinhold Kainhofer
> <address@hidden> wrote:
> > Am Dienstag, 9. September 2008 schrieb Han-Wen Nienhuys:
> > It also automatically includes all changes and does not let you select
> > which changes you want to submit. If you work on a basic issue and
> > something else that is based on it, you can't upload e.g. a patch that
> > you committed locally separately already from some uncommited changes.
>
> Are you sure that git-cl sends uncommittted changes?  I would qualify
> that as a bug.

Yes, that's where the main difference of svn and git comes in: git-cl appears 
to simply upload the diff of the current checkout to a given point/branch. 
In my example, my local git commit contained only the changes to stencil.
{hh,cc} and stencil-scheme.cc, but not the changes to 
input/regression/flags-in-scheme.ly. Still, git-cl uploaded those changes, 
too.

It would also be very useful if git-cl let you select one patch (not 
necessarily the latest, so no diff of current head to some other point) to 
upload. In this case, the description / summary can be taken from the commit 
message.

Cheers,
Reinhold
- -- 
- ------------------------------------------------------------------
Reinhold Kainhofer, Vienna University of Technology, Austria
email: address@hidden, http://reinhold.kainhofer.com/
 * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at/
 * K Desktop Environment, http://www.kde.org, KOrganizer maintainer
 * Chorvereinigung "Jung-Wien", http://www.jung-wien.at/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIx+j1TqjEwhXvPN0RAgGIAJsHpDVohsB0nQYE3RyNFWQS+XfEoQCgx4Yw
P2z5TOYZChSuJAICCwd/xYc=
=zJTs
-----END PGP SIGNATURE-----