[Webkit-unassigned] [Bug 36394] Include git commits in the diff for webkit-patch upload/land.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36394





--- Comment #29 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2010-03-29 21:43:04 PST ---
(In reply to comment #28)
> (In reply to comment #20)
> > I know I'm in the minority, but I don't want webkit-patch squashing my commits
> > or otherwise modifying my git repository, other than to update placeholders for
> > reviewers (or bug numbers) before it lands local commits individually.
> 
> I guess I don't quite understand your workflow still. For every bug you have
> one branch, right? Within that branch you have multiple patches. You them them
> reviewed and the git svn dcommit them, which makes an svn commit for each
> commit. Did I get that right?

Correct.

> If that's the case, how do you test each svn commit separately? If you don't
> test them separately, shouldn't they go in as a single commit? That way, if
> there is a problem with the commit and someone else needs to roll it out (e.g.
> on a release branch), they can safely roll out one atomic unit.

You're missing the point of having separate commits--making reviews easier and
making small, logical changes together.  Each commit should be able to stand on
its own and pass all tests.

There are many ways to run tests against individual commits.  Usually that's
done as you're developing, but using "git rebase -i" and marking each commit as
"edit" would allow you to run tests against each commit.

> > If I wanted commits squashed, I will do it myself.  Likewise I wouldn't have
> > local changes when running webkit-patch--I would have committed them (usually
> > on a branch) before I ran the command.  I tend to have multiple patches in
> > progress on different branches in my git repo, so local changes don't make much
> > sense in that scenario.
> 
> Another possibility Evan suggested, which makes sense to me:
> 
> webkit-patch upload will check if there are multiple local commits and/or
> working copy changes. If there are, it will ask whether to squash them into one
> patch, if you say no, it will upload them as separate patches.
> 
> webkit-patch land warns that there are working copy changes and asks whether to
> commit them. Then it checks if there are multiple local commits and asks
> whether to squash them or not.
> 
> Both commands will also have --git-commit option to upload/land just a single
> local commit and a --squash=true/false option, to avoid the prompts.
> 
> I still think squash should be the default. It matches the concept of commiting
> as an atomic unit the thing you tested.
> 
> This is more ambitious than I was starting with, but this can be done in
> iterations if we agree on the desired end result.

That's fine to make squashing the default.  It would be cool I could set a
default via .gitconfig to make the default not squash for me.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.