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
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
