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

2010-05-16 Thread Ojan Vafai
On Fri, May 14, 2010 at 9:20 PM, David Kilzer ddkil...@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 cjerdo...@webkit.orgwrote:

 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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


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

2010-05-16 Thread Chris Jerdonek
Thanks, Ojan.

On Sun, May 16, 2010 at 8:26 AM, Ojan Vafai o...@chromium.org wrote:
 On Sat, May 15, 2010 at 2:17 PM, Chris Jerdonek cjerdo...@webkit.org
 wrote:
  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.

Currently, when using check-webkit-style for example, I need to pass
--git-commit=HEAD^.. to include the commit at head.  In other words,
it operates on ranges like the above exclusively with respect to the
endpoint.  This is similar to how git-diff behaves and is described
here:

http://www.kernel.org/pub/software/scm/git/docs/git-rev-parse.html

Were you thinking about changing this behavior?

--Chris
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Ancient patches in pending-review

2010-05-16 Thread 鵜飼文敏
On Fri, May 14, 2010 at 02:31, Alexey Proskuryakov a...@webkit.org wrote:


 13.05.2010, в 10:19, David Levin написал(а):


  Should we just r- and ask that it wait for conceptual issues to be
 resolved in IETF spec review first (unless for some reason it is needed
 quickly and that takes too long)?



 I am not aware of any specific conceptual issues, just the fact that TCP
 connection closing is not as simple as some people think. It would take me
 more time to refresh my knowledge than I've been able to afford yet, but
 there is nothing here that should stop another reviewer.


https://bugs.webkit.org/show_bug.cgi?id=35573 just adds CloseEvent, as
http://html5.org/tools/web-apps-tracker?from=4816to=4817

Closing handshake that will set wasClean flag of CloseEvent will be
implemented in https://bugs.webkit.org/show_bug.cgi?id=35721.
Should I put these in one patch?

-- 
ukai




 - WBR, Alexey Proskuryakov

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev