[webkit-dev] Eliminate potential null pointer dereference?

[Ryosuke Niwa]

On Thu, Apr 19, 2012 at 10:35 PM, David Barr <davidbarr at google.com> wrote:
>
> Regarding style, the change homogenizes the loop constructs within that
> method.


I don't think that's necessary an improvement. e.g. we don't have a style
guide that mandates it.

I completely agree with Maciej here that if this is a reachable code, then
the patch author should put a reasonable effort into creating a test case. And
most importantly, these changes are clearly not "code cleanup".

On Thu, Apr 19, 2012 at 11:11 PM, David Levin <levin at google.com> wrote:

> I understand the other side as well that it would be good to figure out if
> it is really an issue and find a test to prove it. I guess this is more of
> what I think of as a BSD type of approach. It seems to be an area where
> reasonable people can disagree.


The WebKit contribution guide lists this as a requirement (
http://www.webkit.org/coding/contributing.html):
For any feature that affects the layout engine, a new regression test must
be constructed. If you provide a patch that fixes a bug, that patch should
also include the addition of a regression test that would fail without the
patch and succeed with the patch. If no regression test is provided, the
reviewer will ask you to revise the patch, so you can save time by
constructing the test up front and making sure it's attached to the bug.

So I don't think we can "disagree" on this topic. I'm sympathetic to the
argument that it's hard to come up with a test case for things like this,
but then the patch author should clearly state that in the change log, and
most importantly the reviewer should be asking that during the review.

- Ryosuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120419/2b1e5965/attachment.html>