Summary: | [Chromium] Delete WebKit/chromium/bridge | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dglazkov, fishd, jamesr, tkent, tommyw, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Adam Barth
2012-03-29 16:53:16 PDT
Created attachment 134695 [details]
Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Attachment 134695 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1
Source/WebCore/platform/chromium/support/WebMediaStreamSourcesRequest.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamSourcesRequest.cpp:38: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaHints.cpp:37: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebICEOptions.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamComponent.cpp:37: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebICECandidateDescriptor.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamSource.cpp:37: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamDescriptor.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamDescriptor.cpp:38: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
Total errors found: 9 in 52 files
If any of these errors are false positives, please file a bug against check-webkit-style.
I like this patch a lot and it can be further simplified by deleting the *Internal classes (to my eternal joy and happiness). I'll be happy to do this in a follow-up patch. There seems to be a major problem with the patch in that the files going to Source/Platform/chromium/public are missing. You could try generating the patch yourself with git diff --no-renames --no-prefix master..HEAD >some_filename and uploading it manually. If you are using git that is. From the actual patch: Index: Source/Platform/chromium/public/WebICECandidateDescriptor.h =================================================================== --- Source/Platform/chromium/public/WebICECandidateDescriptor.h (revision 112571) (from Source/WebKit/chromium/public/platform/WebICECandidateDescriptor.h:112571) +++ Source/Platform/chromium/public/WebICECandidateDescriptor.h (working copy) Index: Source/Platform/chromium/public/WebICEOptions.h =================================================================== --- Source/Platform/chromium/public/WebICEOptions.h (revision 112571) (from Source/WebKit/chromium/public/platform/WebICEOptions.h:112571) +++ Source/Platform/chromium/public/WebICEOptions.h (working copy) that looks fine to me. I'll patch this in locally to review Comment on attachment 134695 [details]
Patch
Looks great! R=me. You've got merge conflicts in the .gypis, but that's not a huge surprise.
Unfortunately, some of the earlier patches were rolled back, so I'll need to figure them out first, Committed r112784: <http://trac.webkit.org/changeset/112784> > Unfortunately, some of the earlier patches were rolled back, so I'll need to figure them out first, I managed to beat the Windows component build into shape and was able to re-land the earlier patches. > I like this patch a lot and it can be further simplified by deleting the *Internal classes (to my eternal joy and happiness). I'll be happy to do this in a follow-up patch. Yay! That's the goal. Getting rid of the "Internal" classes would be quite welcome. Depending on how that works, we might need to do some more build system work. Let me know if you run into any difficulties. |