[webkit-reviews] review denied: [Bug 29722] Chromium Port - DEPS file & webkit.gyp : [Attachment 40089] patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Yaar Schnitman
<yaar at chromium.org>'s request for review:
Bug 29722: Chromium Port - DEPS file & webkit.gyp
https://bugs.webkit.org/show_bug.cgi?id=29722

Attachment 40089: patch
https://bugs.webkit.org/attachment.cgi?id=40089&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Just a few minor nits to address or respond to.


> diff --git a/WebKit/ChangeLog b/WebKit/ChangeLog
> +2009-09-24  Yaar Schnitman  <yaar at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=29722
> +
> +	   * chromium/DEPS: Describes the chromium port's dependencies and
> +	     is used by gclient to fetch them.
> +	   * chromium/webkit.gyp: Currently a thin wrapper around webcore but
in
typo: "in" it?
> +	     soon will build the webkit api.

What do you mean by thin wrapper about webcore?  Do you mean that it only
builds webcore?


> diff --git a/WebKit/chromium/DEPS b/WebKit/chromium/DEPS

> +
> +vars = {
> +  'chromium_svn': 'http://src.chromium.org/svn/trunk/src',
> +  'chromium_deps_svn': 'http://src.chromium.org/svn/trunk/deps/third_party',

> +
> +  # 

Is this suppose to be a comment?


As discussed I have concerns about the repeated revision numbers here but it
sounds like this is on your clean up list real soon now.

> diff --git a/WebKit/chromium/webkit.gyp b/WebKit/chromium/webkit.gyp
> +{
> +  'targets': [
> +    {
> +	 # This target is a thin wrapper around webcore, but it will
Consider "This target only build webcore right now, but it will"