[webkit-dev] Eliminate potential null pointer dereference?

[David Levin]

On Thu, Apr 19, 2012 at 11:36 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> 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.
>

You seem to be a bit confrontational here. I'm not sure why.

I was talking about about doing a patch for something where one wasn't able
to find a repro but it appeared like an issue might be there. Not whether
the changelog should say that or not. It may be good to ask a clarifying
question if something is unclear as opposed to responding like this.

dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120419/3c45a6e7/attachment.html>