[webkit-dev] Current legacy argument refactoring -- behavior changes?

[Adam Barth]

On Fri, Aug 5, 2011 at 1:06 PM, Darin Adler <darin at apple.com> wrote:
> Mark, Adam, thanks for your replies.
>
> It seems obviously OK to do this for new APIs that have not been around long, especially ones that never shipped in a production browser, for the same reason that we decided to do this for new functions from this point forward. I have no quibble with that.
>
> The way we checked in these changes makes it a challenge to figure out what changed. The list you mailed is easier to understand than the changes themselves, but it’s a pretty long list. Normally we’d want to triage, quickly land completely non-controversial changes, and discuss the more interesting cases at greater length.
>
> Best practices for WebKit would be to make sure there are patches where we can see easily the functions affected in each patch. We could accomplish this by first checking in with the [Optional] in a refactoring patch, then removing [Optional] in a behavior change patch along with test cases and, as appropriate, a bit of further discussion discussion.
>
> The reason these are best practices is that they help others involved in the project understand the changes and even participate in decision making. The way we’ve been doing it up until now makes it hard for anyone to follow along. The reason I have reviewed so few of these patches is that it’s very hard for me to figure out which ones include behavior changes.

Yeah, we should have more clearly separated phase (3) and phase (4).
Ideally, we should have sent out a plan to webkit-dev and then used
meta bugs to separate out the different phases so that folks could CC
themselves on the bugs related to whichever phase they were most
interested in.

For the major features that moved to the newer behavior, we discussed
each personally with at one of the folks who is a major contributor to
the feature.  Not all those discussions are recorded in the bug
tracker because some of them happened in person or on #webkit.

> What about my test coverage question?
>
> Normally we require tests when changing WebKit behavior. Is there a reason these changes are an exception? It seems that in most of these cases creating a test would be straightforward and it would be useful for the project to have such tests.

I've filed https://bugs.webkit.org/show_bug.cgi?id=65787 about adding
the missing tests.

Thanks,
Adam