[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:06:08 PST 2010


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





--- Comment #9 from David Levin <levin at chromium.org>  2010-02-11 08:06:04 PST ---
(In reply to comment #8)
> For the record, I'm not sure I agree.  Making the build logic more complicated

> in order to save a couple lines of pre-processor here and there really seems
> like a net loss.  Especially since any WebKit developer understands the "#if
> ENABLE" logic but not necessarily the GYP/Makefile logic.

You're saying that developers don't know how to add a file to the build? If so,
that is really broken.

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

This looks like a really trivial one line change and much simpler that adding
if ENABLE's  in lots of different filfes to exclude some header file, so I
don't get why this is worse.

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

This seems like standard boiler plate in the makefile for idl files. It is a
shame that it requires so much boilerplate but that's the way these particular
build files are.

> seem a lot worse than simply adding these guards whenever we're including files
> that are generated.

fwiw, the guidelines against doing if ENABLE's for headers is a common
recommendation in WebKit reviews. It is more confusing if folks need to
remember don't do this except when the included file is autogenerated through
some process (every additional rule/exception is more mental overhead).

Lastly, we should strive to make source files as clean and easy to read as
possible and not having unnecessary if ENABLEs fits nicely into that goal. (And
they are unnecessary with the simple makefile additions.)

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