[webkit-dev] webkit-patch, check-webkit-style and git now support --squash and --git-commit

[Ojan Vafai]

On Fri, May 14, 2010 at 9:20 PM, David Kilzer <ddkilzer at webkit.org> wrote:

> Personally, I don't see why both models (single patches and multiple
> patches) couldn't be supported based on the arguments passed to
> webkit-patch.  git itself seems to be able to figure out if the user is
> referring to a single commit or a series of commits (based on the context of
> the command-line arguments and what the command does), so I don't see why we
> have to introduce additional command-line switches for various behavior.
> (See the git-send-email command as an example since it's designed to send
> email for a single patch or a series of patches.)
>
> In my opinion, webkit-patch should default to single commit behavior unless
> the user specifies a range.  That way, git neophytes and the majority of
> existing users get the single-commit-to-a-single-bug behavior by default.
>  However, if a range of commits is given, then webkit-patch would be smart
> enough to post the series of patches to a single bug.
>

Right now, if you pass a commit-range it squashes all the commits into a
single commit. I don't think it would be a big deal to change that behavior.
The harder part is that webkit-patch is entirely built around single
commits, both from API and code. For example, it assumes a single reviewer
and commit message for the whole command. Changing that would be a lot of
work.

These are certainly solvable problems, but it's not clear to me that it's
worth trying to shoe-horn this use-case into webkit-patch. The more I think
about this, the more I think we should use Chris's proposal (detailed below)
and write a separate script for dealing with commit-ranges in that way.
webkit-patch is much simpler if we keep it's semantics simple and writing a
script that calls webkit-patch in a loop and only exposes the sensible
options is relatively easy.

On Sat, May 15, 2010 at 2:17 PM, Chris Jerdonek <cjerdonek at webkit.org>wrote:

> It would support a commit-per-bug workflow.  Just not committing a patch
>
series all at once with a single invocation of webkit-patch (at least
> for now -- see below).
>

Yup. I think that's the key point here and having a separate script
alleviates the tedium of needing to manually call webkit-patch multiple
times.


> Personally, I think webkit-patch and check-webkit-style should include
> unstaged changes when using the ".." syntax.


This make sense to me.


>  I also don't think it
> would be important for the scripts to make a distinction between unstaged
> and staged changes -- in part because svn-apply does not behave uniformly
> in this regard.


I agree.


>  In particular, --git-commit=HEAD.. should be just the
> uncommitted changes (staged and unstaged).
>

This one I'm a bit iffy on. Should this include the commit at head? I think
it should.

I propose:
default (--git-commit=..): operate on uncommitted changes. errors out if
there are committed changes and --git-commit wasn't explicitly passed in
--git-commit=head..  operate on uncommitted changes + head as a single
commit
--git-commit=head~3  operate on just head~3
--git-commit=head~4..head~2  operate on head~4, head~3 and head~2 as a
single commit
--git-commit=*  operate on all changes in your branch as a single commit

Then we get rid of --squash and --no-squash.

Ojan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100516/662be589/attachment.html>