Summary: | Chromium Port - DEPS file & webkit.gyp | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yaar Schnitman <yaar> | ||||||||
Component: | WebKit Misc. | Assignee: | Yaar Schnitman <yaar> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, dglazkov, eric, levin, mark | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 28396, 29749 | ||||||||||
Attachments: |
|
Description
Yaar Schnitman
2009-09-24 14:42:18 PDT
Created attachment 40089 [details]
patch
Comment on attachment 40089 [details] patch Just a few minor nits to address or respond to. > diff --git a/WebKit/ChangeLog b/WebKit/ChangeLog > +2009-09-24 Yaar Schnitman <yaar@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" Created attachment 40149 [details]
updated patch
Thanks for the feedback. I incorporated it in. I also updated some revision numbers in DEPS to catch up with lkgr chromium.
Comment on attachment 40149 [details] updated patch Rejecting patch 40149 from commit-queue. Patch https://bugs.webkit.org/attachment.cgi?id=40149 from bug 29722 failed to download and apply. Comment on attachment 40149 [details]
updated patch
Trying to apply patch again (I suspect it was rejected due to flakiness).
Comment on attachment 40149 [details] updated patch Rejecting patch 40149 from commit-queue. yaar@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py. Comment on attachment 40149 [details]
updated patch
Oops, I guess I have to do ? rather than +.
Comment on attachment 40149 [details] updated patch Rejecting patch 40149 from commit-queue. Patch https://bugs.webkit.org/attachment.cgi?id=40149 from bug 29722 failed to download and apply. Created attachment 40248 [details]
Fixed build rejection problems
Previous patch was already r+, cq+, but in the meantime patch got rotten. This is patch just solves the merge problems.
Comment on attachment 40248 [details]
Fixed build rejection problems
adding back to cq, carrying over levin's r+.
patching file WebCore/WebCore.gyp/WebCore.gyp Hunk #1 FAILED at 35. Hunk #2 succeeded at 419 (offset 4 lines). 1 out of 2 hunks FAILED -- saving rejects to file WebCore/WebCore.gyp/WebCore.gyp.rej was the original rejection. Comment on attachment 40248 [details] Fixed build rejection problems Clearing flags on attachment: 40248 Committed r48827: <http://trac.webkit.org/changeset/48827> All reviewed patches have been landed. Closing bug. |