Summary: | [Qt] Single-threaded QtWebKit configuration | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Laszlo Gombos <laszlo.gombos> | ||||||||||
Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ariya.hidayat | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Laszlo Gombos
2009-05-25 19:02:30 PDT
Created attachment 30662 [details]
Proposed patch.
ENABLE_NO_THREADING is an awkward name. Why not DISABLE_THREADING, or set ENABLE_THREADING=0? (In reply to comment #2) > ENABLE_NO_THREADING is an awkward name. Why not DISABLE_THREADING, or set > ENABLE_THREADING=0? The guidelines I set to myself - stay consistent with the ENABLE_XXX naming schema - the build should not change if the newly introduced flag is not set to minimize the impact of this less frequently used configuration. How about ENABLE_SINGLE_THREADED ? Created attachment 30838 [details]
Updated the patch to use ENABLE_SINGLE_THREADED as the guard.
Landed in 44411 Created attachment 31026 [details]
Fix compilation warnings in ThreadingNone.cpp
It seems to me that changes to ThreadingNone.cpp has not been committed as part of 44411; created a new patch just for those changes.
Reopen as part of https://bugs.webkit.org/attachment.cgi?id=30838 has not been committed as part of 44411. I prefer that we use UNUSED_PARAM macro. The argument names are always useful to help understanding the function. Created attachment 31062 [details]
Use UNUSED_PARAM macro for unused arguments as Ariya suggested.
(In reply to comment #8) > I prefer that we use UNUSED_PARAM macro. The argument names are always useful > to help understanding the function. Hehe, it is a matter of taste. In general we omit parameter names when it is obvious. I would argue that for a *None file the parameter names are not relevant at all and should be omitted. Something funny I stumbled across is in in UnusedParam.h: /* don't use this for C++, it should only be used in plain C files or ObjC methods, where leaving off the parameter name is not allowed. */ #define UNUSED_PARAM(x) (void)x we obviously don't stick to that rule... I share Zecke's opinion for *None files so I canceled the review for https://bugs.webkit.org/attachment.cgi?id=31062. Closing it, since the issue is settled (In reply to comment #12) > Closing it, since the issue is settled Ariya, I think we should still commit https://bugs.webkit.org/attachment.cgi?id=31026 before closing this bug. Change the status back to REOPEN. Oops, sorry. I didn't read it carefully. Landed in r44607 http://trac.webkit.org/changeset/44607 |