[webkit-reviews] review denied: [Bug 31325] [Android] Android port lacks Makefiles : [Attachment 43119] Patch 1 for Bug 31325

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 13 10:33:36 PST 2009


Sam Weinig <sam at webkit.org> has denied Steve Block <steveblock at google.com>'s
request for review:
Bug 31325: [Android] Android port lacks Makefiles
https://bugs.webkit.org/show_bug.cgi?id=31325

Attachment 43119: Patch 1 for Bug 31325
https://bugs.webkit.org/attachment.cgi?id=43119&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
There seems to be a lot of duplication between the *jsc.mk and *.v8.mk files. 
That should really be avoided since we really don't want to have to update both
when adding new files.	The copyright date also seems out of date.


> Index: WebCore/WebCorePrefix.h
> ===================================================================
> --- WebCore/WebCorePrefix.h	(revision 50854)
> +++ WebCore/WebCorePrefix.h	(working copy)
> @@ -62,6 +62,18 @@
>  #include <pthread.h>
>  #endif // defined(WIN32) || defined(_WIN32)
>  
> +#if defined(ANDROID)
> +#ifdef __cplusplus
> +// Must come before include of alorithm.
> +#define PREFIX_FOR_WEBCORE 1
> +#define EXPORT __attribute__((visibility("default")))
> +#endif
> +// We use a single set of include directories when building WebKit and
> +// JavaScriptCore. Since WebCore/ is included before JavaScriptCore/, we
include
> +// JavaScriptCore/config.h explicitly here to make sure it gets picked up.

The "we" is unclear here.  You should probably use a more descriptive term like
"Android".


r- to address the duplication.


More information about the webkit-reviews mailing list