[Webkit-unassigned] [Bug 28673] Modifying <text rotate=""> doesn't clear the corresponding SVGAnimatedNumberList

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28673





--- Comment #3 from Cameron McCormack <cam at mcc.id.au>  2009-08-23 22:30:10 PDT ---
(In reply to comment #2)
> If you're not commenting on these functions individually and the file is new,
> then there's no need to leave this list of functions in the change log. The
> idea of prepare-ChangeLog is that it helps you write the ChangeLog entry, not
> that you should use that it generates as-is.

I will keep that in mind for future patches.

> The test code doesn't follow our usual brace style.

OK.  I see there's no mention of JS style in
http://webkit.org/coding/coding-style.html but I'll admit I didn't check it
before writing the code.

> Open braces for functions
> go on their own lines. Single line for loop bodies don't get braces around
> them. I would have put the "var" for the "i" variable inside the for loop.

I used to too, but now tend to put vars at the top of functions to emphasise
that their scope is indeed the whole function and not just the loop.

> I believe the function results could be arrays rather than strings -- the
> shouldBe macro knows how to check arrays for equality.

Acknowledged.  I didn't look up shouldBe to see exactly what it was checking; I
just copied another test that was doing string comparisons and thought that it
was just safer to do that.

> We don't use "get" in the names of functions that simply return results.

I guess that's Java style rubbing off on me. :)

> None of these are significant issues, r=me on this patch as it is.

Thanks!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.