[Webkit-unassigned] [Bug 34812] [Android] SVG code in V8 bindings is not properly guarded

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 11 08:44:50 PST 2010


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





--- Comment #12 from David Levin <levin at chromium.org>  2010-02-11 08:44:48 PST ---
(In reply to comment #11)
> I don't know how to add a file to the android build, no.  Bindings are
> especially mysterious across ports.

Well that is not the case here because Steve Block does know but it is a much
bigger problem. (Also, when these items were added, they weren't ifdef'ed
anyway.)

> #if ENABLE in the file.  But if the file might not exist, making the build
> system more complicated and having to do the change in multiple places is not
> worth it.

The build isn't more complicated. In one change he added a *single* line. That
is certainly not more complicated than one he did before. In the other case, he
copied some boilerplate. Is that complicated boilerplate? Maybe, but it is just
boilerplate not some complicated addition to the build system. (Perhaps some
effort could be put into making the boilerplate simpler if Android finds this
to be a problem.)

> And really. In the top of a hand full of files like that, do you really find a
> few #if ENABLE so distracting?  I'm not sure I believe that.

Readability is the result of one thing but a lot of little things. Sure this is
a little thing we could compromise on something that makes the file less
readable. Do this enough and your file becomes a lot less readable.

Lastly, you're ignoring the mental effort from keeping exception in mind for
developers. "Don't do if ENABLE's for headers unless the if ENABLE is for a
file that is autogenerated. Is this header file autogenerated?" Instead of
simply "Don't do if ENABLE's for headers."

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



More information about the webkit-unassigned mailing list