| Summary: | [PAL] Move spi/cf directory into PAL | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yoshiaki Jitsukawa <yoshiaki.jitsukawa> | ||||||||||||||||
| Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | achristensen, buildbot, commit-queue, don.olmstead, koivisto, mmaxfield, webkit-bug-importer, yoshiaki.jitsukawa | ||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Yoshiaki Jitsukawa
2017-08-01 16:10:45 PDT
Moving all the SPI headers at once tends to cause merge conflict so I'm trying one directory by one. Created attachment 316910 [details]
Patch
Attachment 316910 [details] did not pass style-queue:
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42: _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:43: _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFLocaleSPI.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CoreMediaSPI.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5]
Total errors found: 5 in 72 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Do we need to fix style of SPI headers? Created attachment 316917 [details]
Patch
I forgot to fix sources under the "Tools" directory...
Attachment 316917 [details] did not pass style-queue:
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42: _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:43: _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFLocaleSPI.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CoreMediaSPI.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5]
Total errors found: 5 in 74 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Looks good once you fix the style check. Just the pragma once correct? Created attachment 316942 [details]
Patch
Fixed the pragma once. I'm not sure whether the function names should be fixed or not...
Attachment 316942 [details] did not pass style-queue:
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:41: _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42: _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 74 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 316948 [details]
Patch
Add a path-specific(WebCore/PAL/pal/spi) style rules to ignore underscores in identifier names.
Comment on attachment 316948 [details] Patch Attachment 316948 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4239366 New failing tests: fast/dom/HTMLTemplateElement/ownerDocumentXHTML.xhtml Created attachment 316955 [details]
Archive of layout-test-results from ews113 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 316990 [details]
Patch
(In reply to Myles C. Maxfield from comment #7) > Looks good once you fix the style check. I've fixed the style check - Fix the include guards - Add a path-specific style rule for WebCore/PAL/pal/spi to ignore [readability/naming/underscores] Would you review my patch? The test failure on mac-debug EWS seems to be a false positive. Comment on attachment 316990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316990&action=review It looks like there are a lot of unrelated changes in the Xcode project files. Are these intentional? Also, because this patch touches the WebKit sub-project, you’ll need an OWNER to review those pieces. Other than that, this looks good. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137 > - CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */, Can you make sure you run the sort-Xcode script in Tools/Scripts on this project? (In reply to Myles C. Maxfield from comment #16) > Comment on attachment 316990 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316990&action=review > > It looks like there are a lot of unrelated changes in the Xcode project > files. Are these intentional? > > Also, because this patch touches the WebKit sub-project, you’ll need an > OWNER to review those pieces. > > Other than that, this looks good. > > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137 > > - CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */, > > Can you make sure you run the sort-Xcode script in Tools/Scripts on this > project? Thank you. I CC Alex from the Owners list. I ran the sort script and unfortunately those unrelated diff were made by it. (In reply to Yoshiaki Jitsukawa from comment #17) > (In reply to Myles C. Maxfield from comment #16) > > Comment on attachment 316990 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=316990&action=review > > > > It looks like there are a lot of unrelated changes in the Xcode project > > files. Are these intentional? > > > > Also, because this patch touches the WebKit sub-project, you’ll need an > > OWNER to review those pieces. > > > > Other than that, this looks good. > > > > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137 > > > - CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */, > > > > Can you make sure you run the sort-Xcode script in Tools/Scripts on this > > project? > > Thank you. I CC Alex from the Owners list. > > I ran the sort script and unfortunately those unrelated diff were made by it. Can we sort the file in a preliminary separate patch? (In reply to Myles C. Maxfield from comment #18) > Can we sort the file in a preliminary separate patch? Sure. Let me try. (In reply to Yoshiaki Jitsukawa from comment #19) > (In reply to Myles C. Maxfield from comment #18) > > Can we sort the file in a preliminary separate patch? > > Sure. Let me try. I've filed these bugs and sent patches. Bug 175121 - [WebCore] Sort Xcode project files Bug 175122 - [WebKit] Sort Xcode project file (just in case) Created attachment 317160 [details]
Patch
Comment on attachment 317160 [details]
Patch
Not sure whats up with ios-sim but it seems like its flaky in other patches we've submitted today.
Comment on attachment 317160 [details] Patch Clearing flags on attachment: 317160 Committed r220243: <http://trac.webkit.org/changeset/220243> All reviewed patches have been landed. Closing bug. |