Thanks for your responses, Ojan. On Fri, May 14, 2010 at 6:29 PM, Ojan Vafai <o...@chromium.org> wrote: >> Maybe it already does this, but would it be possible to make the default >> behavior be to throw an error if there is more than one possibility for >> what should be committed? > > That's what it currently does. > >> It seems like this would reduce the chance of >> accidentally committing the wrong information. A configuration option >> could still be used to override this behavior for those who know they want >> to use one of the two behaviors exclusively. > > I think the --squash behavior is actually pretty simple. It's a lot like > svn. It commits everything in your branch. End of story. Now > that https://bugs.webkit.org/show_bug.cgi?id=38852 is almost fixed, it's > much safer as well.
Yes, it's simple. My point was that it may be safer if the script did not commit everything by default. That way we avoid accidental over-commits if one forgets to type --git-commit or overlooks that their branch contains multiple patches. >> I also suggest this because it's not clear to me that there are only two >> camps. For example, I maintain one branch per bug (branch-per-bug), but I >> will often create a branch from one of these when working on a bug that >> depends on another of my bugs being landed (commit-per-bug). So I can see >> myself using both options. > > I don't actually see what the third option here is. I'd call both of those > approaches branch-per-bug. You'll commit the first branch using --squash and > then you can commit the second branch with --squash. That said, I find > myself using --squash ~80% of the time and --git-commit the other 20%. > Changing the default or the webkit-patch.squash value does not preclude you > using another value via the command-line (i.e. command-line overrides both). In my workflow I maintain one branch per bug. I also maintain one commit per bug because I commit locally using --amend. So normally I would not have a need for --squash. In addition, when I am working on sequential patches, there are points in my workflow where I have multiple uncommitted bugs on a single branch. My main reason for describing this scenario was to illustrate a workflow where defaulting to committing everything would increase the likelihood of an accidental over-commit (since I sometimes have multiple bugs on a branch). >> Finally, I'm wondering if the three options above could perhaps be >> simplified >> to a single option if we made it clear that webkit-patch supports working >> with only one revision at a time. > > I agree, but you're essentially saying everyone should use branch-per-bug. > There are enough people who specifically use git for the commit-per-bug > approach that I don't think webkit-patch will meet everyone's needs if we do > this. 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). >> If webkit-patch supported only one revision at a time, then it seems like >> the three options could be reduced to a single --git-commit option, which >> would specify which commits should go into creating the single >> revision/patch. > > I would be very happy with this. It would greatly simplify webkit-patch > (both the code and usage). However, we're basically saying we won't > explicitly support commit-per-bug workflows. If someone wanted to > upload/commit a range of commits as separate patches, they need to do each > one as a separate command. That said, people who want to do commit-per-bug > workflow could easily write a script that calls webkit-patch in a loop for > each commit. Yes, adding support via looping is what I had in mind (at least initially). That way we can focus on simplifying and nailing down the semantics for the single-patch use case before working out the semantics of committing multiple patches at once. > There's a couple edge cases that are unclear to me. a) How do you upload > just the working copy? Is that the default? b) Does --git-commit=* include > the working copy? I think it should. For check-webkit-style, I believe all uncommitted changes used to be the default (staged and unstaged). check-webkit-style also used to include unstaged changes when using the --git-commit syntax (now it doesn't). Personally, I think webkit-patch and check-webkit-style should include unstaged changes when using the ".." syntax. 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. In particular, --git-commit=HEAD.. should be just the uncommitted changes (staged and unstaged). --Chris _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
