[webkit-dev] Yet another bug-less change hosed the tree.

Adam Barth abarth at webkit.org
Mon May 10 14:44:58 PDT 2010


On Mon, May 10, 2010 at 2:31 PM, Geoffrey Garen <ggaren at apple.com> wrote:
>> If you'll allow me to climb higher on my horse, I'll advocate for
>> landing patches like this through the commit queue.
>
> Personally, I find the output of the commit queue incredibly confusing, since it claims that Eric Seidel wrote every patch.

Indeed.  Fortunately, we know how to solve that problem.  It just
requires a post-commit hook to change the svn author to match the
ChangeLog.  I think Eric already wrote the hook.  We just need someone
on the server side to install it.

On Mon, May 10, 2010 at 2:30 PM, Geoffrey Garen <ggaren at apple.com> wrote:
>>> What value would a bug report add in a situation
>>> like http://trac.webkit.org/changeset/59086?
>>
>> Having a bug serves a couple useful purposes:
>>
>> 1) Your patch can be reviewed by other members of the project who
>> aren't sitting next to you on couch.
>
> I don't think anyone else had a review comment about this patch. Did they?

We'll never know.  However, in this case, the issue appear to be more (2) below.

>> 2) Your patch can be vetted by the various bots that analyze patches
>> posted for review.
>
> True, if what you're really asking for is not just a bug report but also a "cooling off period" during which you wait for a result from the EWS bot, even if you get a review right away. You get greater value in the case of a bad patch, but also greater cost in the case of every patch.

Yes, this way of doing things has more overhead for you personally but
saves overhead for everyone else in the project.  The question, as I
see it, is which of these quantities is larger.  The more people that
work on the project, the bigger the multiplier on the right.

In either case, we can improve the tools to make the costs on you as
small as possible.

>> 3) The but serves as a coordination point for dealing with bustage
>> caused by a patch and for regressions detected later.
>
> Isn't it just as easy to open a bug on demand, if regressions are detected?

We often do that too.  However, if I notice that something went wrong
with a patch, it's helpful to have a central point of contact to look
and see whether someone else noticed it already.  I often do that by
checking the initial bug report to see if it has been reopened, etc.
I can file a new bug, but it might be a dupe.

>>> Personally, I'd prefer it if we kept the TPS reports in our project to a
>>> minimum. In this case, a TPS report would have been more typing than the
>>> patch itself.
>>
>> That's part of what motivated us to create webkit-patch, which makes
>> creating and managing bugs and patches quite easy.
>
> Maybe I would use webkit-patch if it really were quite easy. I tried it in earnest for a while, but I had to give it up because:
> * I couldn't find a ChangeLog workflow that fit its demands, so using it was actually more complicated than doing everything by hand

Can you explain this in more detail?  I've optimized the tool for my
workflow because that's the workflow I understand.  I'm happy to
optimize it for other workflows too.

> * It didn't handle subdirectory-only work

I think here you're refer to the fact that SVN is slow.  There was a
thread earlier about making webkit-patch operate on the current
directory instead of on whole working copy.  We can certainly do that
if you'd find that helpful.

> * It often failed with mysterious error messages

We've been fixing these as we've encounter them.  Feel free to file a
bug and CC me with the error message you find mysterious and we can
improve it.

>> When you go cowboy and commit without
>> building and testing, you impose costs on everyone else in the
>> project.
>
> Probably not fair to conflate shooting a six-shooter with committing without filling out a bugzilla form first.

In this case, I was talking about committing with build or testing,
which I'm not saying you did with this patch.

Adam


More information about the webkit-dev mailing list