[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