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

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Jeremy Orlow <jorlow at chromium.org>  2010-02-11 08:50:35 PST ---
(In reply to comment #12)
> (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.)

The build is complicated in that only people on that particular port usually
fully understand what's going on.  That means, with the model you're
supporting, each port needs to fix their own build whenever a new conditional
IDL file is added.  Or the developer, if they're nice, need to learn all the
build systems enough to do the fix themselves.  This can often lead to them
doing things subtly incorrectly though, and causing even more headache.

On the other hand, any WebKit committer should understand the basics of #if
ENABLEs.

> > 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.

This is a pretty unique corner case I think.

> 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."

Ports already end up needing to fix things themselves when their build fixes. 
Why not let them fix up the source (which fixes other ports that might have the
same problem...including the original port if they ever opt to turn the feature
off) rather than just putting a fix into their own build system.

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