[Webkit-unassigned] [Bug 51789] Don't include Inspector headers when Inspector is disabled

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jan 2 02:09:37 PST 2011


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





--- Comment #10 from Alexey Proskuryakov <ap at webkit.org>  2011-01-02 02:09:36 PST ---
Could you tell me more about how this change helps you?

What I'm saying is that our normal approach is to have checks like #if ENABLE(INSPECTOR) in header files, not in .cpp files. So, the header becomes effectively empty when a feature is disabled.

See e.g. loader/appcache/ApplicationCache.h for how we normally do this.

There are multiple benefits in doing it this way:
- .cpp files are much easier to read and write, because headers are in simple alphabetical order, not in three-line blocks per feature define;
- it's somewhat easier to avoid breaking the build, because #if guards are likely added earlier in development cycle, when everyone remembers that the feature is optional;
- we can start having unrelated includes in headers outside #if guards - as mentioned in my previous comment, that would also make breaking the build less likely. But that's not something we do today.

As far as I can tell, adding proper #if guards to header files would be fine for your use case, too.

> Not so big change, as it may seem.

I'm saying that it's big because it touches many files, and thus noticeably increases the chance that someone will copy the wrong idiom from one of them. We currently have a mix of the two approaches, and should move to standardizing on one.

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