Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2846463002:
Add missing header files to GN files (4 of N) (Closed)
|
Created:
3 years, 8 months ago by wychen Modified:
3 years, 7 months ago Reviewers:
Nico CC:
chromium-reviews, extensions-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, jam, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin-cc_chromium.org, blink-reviews, chromium-apps-reviews_chromium.org, darin (slow to review), kinuko+watch Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd missing header files to GN files (4 of N)
I ran check_gn_headers.py on linux and android full builds, merged the
missing header file lists, fed to fix_gn_headers.py (without
AddHeadersToSources), chose which matches to apply interactively, and
manually fixed the results.
Read more about the manual fixes in the Review-Url.
The number of missing header files on linux and android decreased by 9,
from 840 to 831.
None of the fixed files are included in the following spreadsheet:
https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0
fix_gn_headers.py is from https://codereview.chromium.org/2790563003
BUG=661774
Review-Url: https://codereview.chromium.org/2846463002
Cr-Commit-Position: refs/heads/master@{#468192}
Committed: https://chromium.googlesource.com/chromium/src/+/fd60d8ca4a34a9cacfa3f45d65598165543fa74e
Patch Set 1 : run script interactively #Patch Set 2 : apply manual fixes from https://crrev.com/2784393003, fix WebServiceWorkerStreamHandle.h #Patch Set 3 : fix WebFontRendering.h #Patch Set 4 : fix WebFontRenderStyle.h #Patch Set 5 : fix webfont files #
Total comments: 3
Patch Set 6 : rebase #Patch Set 7 : address comments #
Total comments: 4
Patch Set 8 : address comments #Patch Set 9 : revert content/common/DEPS, rebase #
Messages
Total messages: 33 (22 generated)
Patchset #1 (id:1) has been deleted
Description was changed from ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 9, from 849 to 840. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ========== to ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 10, from 852 to 842. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ==========
The CQ bit was checked by wychen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by wychen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
wychen@chromium.org changed reviewers: + thakis@chromium.org
PTAL
https://codereview.chromium.org/2846463002/diff/100001/ui/gfx/BUILD.gn File ui/gfx/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/100001/ui/gfx/BUILD.gn#newcod... ui/gfx/BUILD.gn:293: sources += [ "platform_font_linux.h" ] Ah. I'll fix this.
https://codereview.chromium.org/2846463002/diff/100001/services/device/public... File services/device/public/cpp/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/100001/services/device/public... services/device/public/cpp/BUILD.gn:9: "device_features.h", Why is this listed twice? Shouldn't once be enough?
The CQ bit was checked by wychen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 10, from 852 to 842. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ========== to ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 9, from 843 to 834. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ==========
https://codereview.chromium.org/2846463002/diff/100001/services/device/public... File services/device/public/cpp/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/100001/services/device/public... services/device/public/cpp/BUILD.gn:9: "device_features.h", On 2017/04/26 14:29:43, Nico wrote: > Why is this listed twice? Shouldn't once be enough? Oh, my checking script only takes |sources|. I'll amend it to include |public|.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with the two "../../public/platform/linux/WebFontRenderStyle.h" removed https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/BUILD.gn:1567: "../../public/platform/linux/WebFontRenderStyle.h", It's weird to me that source/platform would include a header from public/ -- shouldn't this be in WebKit/public/BUILD.gn ? ...and indeed, there it is: https://cs.chromium.org/chromium/src/third_party/WebKit/public/BUILD.gn?type=... Why is this needed? If it's needed I feel it should be in a separate CL since it's kind of weird, while the rest of this CL is pretty clear. https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/web/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/web/BUILD.gn:258: "../../public/web/linux/WebFontRendering.h", Same here.
https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/BUILD.gn:1567: "../../public/platform/linux/WebFontRenderStyle.h", On 2017/04/27 14:18:09, Nico wrote: > It's weird to me that source/platform would include a header from public/ -- > shouldn't this be in WebKit/public/BUILD.gn ? ...and indeed, there it is: > https://cs.chromium.org/chromium/src/third_party/WebKit/public/BUILD.gn?type=... > > Why is this needed? If it's needed I feel it should be in a separate CL since > it's kind of weird, while the rest of this CL is pretty clear. Moved to WebKit/public. Thanks for catching this! https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/web/BUILD.gn (right): https://codereview.chromium.org/2846463002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/web/BUILD.gn:258: "../../public/web/linux/WebFontRendering.h", On 2017/04/27 14:18:09, Nico wrote: > Same here. This one can be removed. Thanks for catching this!
The CQ bit was checked by thakis@chromium.org
lgtm
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 9, from 843 to 834. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ========== to ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 9, from 840 to 831. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ==========
The CQ bit was checked by wychen@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 180001, "attempt_start_ts": 1493423121966270,
"parent_rev": "65790e805f1e91700e63a2d47fa960105f13648d", "commit_rev":
"fd60d8ca4a34a9cacfa3f45d65598165543fa74e"}
Message was sent while issue was closed.
Description was changed from ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 9, from 840 to 831. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 ========== to ========== Add missing header files to GN files (4 of N) I ran check_gn_headers.py on linux and android full builds, merged the missing header file lists, fed to fix_gn_headers.py (without AddHeadersToSources), chose which matches to apply interactively, and manually fixed the results. Read more about the manual fixes in the Review-Url. The number of missing header files on linux and android decreased by 9, from 840 to 831. None of the fixed files are included in the following spreadsheet: https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0y... fix_gn_headers.py is from https://codereview.chromium.org/2790563003 BUG=661774 Review-Url: https://codereview.chromium.org/2846463002 Cr-Commit-Position: refs/heads/master@{#468192} Committed: https://chromium.googlesource.com/chromium/src/+/fd60d8ca4a34a9cacfa3f45d6559... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:180001) as https://chromium.googlesource.com/chromium/src/+/fd60d8ca4a34a9cacfa3f45d6559... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
