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

[Darin Adler]

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.

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.

    -- Darin