[webkit-reviews] review granted: [Bug 25109] Eliminate CompositeAnimationPrivate : [Attachment 29359] Patch, changelog

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 9 08:53:44 PDT 2009


Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 25109: Eliminate CompositeAnimationPrivate
https://bugs.webkit.org/show_bug.cgi?id=25109

Attachment 29359: Patch, changelog
https://bugs.webkit.org/attachment.cgi?id=29359&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +   
m_compAnim->animationControllerPriv()->removeFromStyleAvailableWaitList(this);
> +   
m_compAnim->animationControllerPriv()->removeFromStartTimeResponseWaitList(this
);

The function name animationControllerPriv() is both long and abbreviated. And I
find it ugly. Can you give it a shorter name without abbreviations? Or I guess
you could use our standard name for such things, impl(), which I also find
ugly.

Aside from this new function that needs a better name, r=me

There are quite a few things here that aren't using standard WebKit style,
using "get" for example for simple getters, whereas we use nouns for getters
and "get" only for getters that use out arguments. And this design with the
private object that you get to explicitly is also pretty awkward.


More information about the webkit-reviews mailing list