|
|
Chromium Code Reviews|
Created:
4 years, 8 months ago by leonhsl(Using Gerrit) Modified:
4 years, 8 months ago Reviewers:
sky CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd deps profile_reset_report_proto for chrome/browser/ui build.
gn gen out/Release
ninja -C out/Release chrome/browser/ui
-->Error generated.
BUG=604186
Committed: https://crrev.com/9660bb7e1bba382d14ca47f14495e934b05f77f2
Cr-Commit-Position: refs/heads/master@{#389030}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase only #Patch Set 3 : Add gyp #
Messages
Total messages: 21 (8 generated)
The CQ bit was checked by leon.han@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1901493002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1901493002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
leon.han@intel.com changed reviewers: + sky@chromium.org
Would you PTAL at this? Thanks~
https://codereview.chromium.org/1901493002/diff/1/chrome/browser/ui/BUILD.gn File chrome/browser/ui/BUILD.gn (right): https://codereview.chromium.org/1901493002/diff/1/chrome/browser/ui/BUILD.gn#... chrome/browser/ui/BUILD.gn:197: "//chrome/browser/profile_resetter:profile_reset_report_proto", Shouldn't this be in the section !is_android and !is_ios? AFAICT that matches the gyp side. Also, isn't the gyp version in the browser sourceset, not the ui sourceset?
The CQ bit was checked by leon.han@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1901493002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1901493002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I checked around profile_reset_report_proto deps for both GN and GYP, uploaded ps#3 to address comments, PTAL again, Thanks~ https://codereview.chromium.org/1901493002/diff/1/chrome/browser/ui/BUILD.gn File chrome/browser/ui/BUILD.gn (right): https://codereview.chromium.org/1901493002/diff/1/chrome/browser/ui/BUILD.gn#... chrome/browser/ui/BUILD.gn:197: "//chrome/browser/profile_resetter:profile_reset_report_proto", On 2016/04/18 15:39:21, sky wrote: > Shouldn't this be in the section !is_android and !is_ios? AFAICT that matches > the gyp side. Also, isn't the gyp version in the browser sourceset, not the ui > sourceset? GN: chrome/browser: Some code files in chrome_browser_non_mobile_sources depend on profile_reset_report_proto, so it is put into the deps to build chrome_browser_non_mobile_sources in chrome/browser/BUILD.gn. It's correct now. chrome/browser/ui: Some code files in chrome_browser_ui_non_mobile_sources depend on profile_reset_report_proto, so, be the same, we should put profile_reset_report_proto into the deps to build chrome_browser_ui_non_mobile_sources. This is what this CL ps1 does. GYP: chrome/chrome_browser.gypi: profile_reset_report_proto is put into the deps to build chrome_browser_non_mobile_sources. It's correct now. chrome/chrome_browser_ui.gypi: profile_reset_report_proto is missing in the deps to build chrome_browser_ui_non_mobile_sources. This CL ps3 added it.
Description was changed from ========== Fix GN build target chrome/browser/ui failure. gn gen out/Release ninja -C out/Release chrome/browser/ui -->Error generated. BUG=604186 ========== to ========== Add deps profile_reset_report_proto for chrome/browser/ui build. gn gen out/Release ninja -C out/Release chrome/browser/ui -->Error generated. BUG=604186 ==========
soft ping, Thanks~
LGTM
The CQ bit was checked by leon.han@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1901493002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1901493002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add deps profile_reset_report_proto for chrome/browser/ui build. gn gen out/Release ninja -C out/Release chrome/browser/ui -->Error generated. BUG=604186 ========== to ========== Add deps profile_reset_report_proto for chrome/browser/ui build. gn gen out/Release ninja -C out/Release chrome/browser/ui -->Error generated. BUG=604186 Committed: https://crrev.com/9660bb7e1bba382d14ca47f14495e934b05f77f2 Cr-Commit-Position: refs/heads/master@{#389030} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/9660bb7e1bba382d14ca47f14495e934b05f77f2 Cr-Commit-Position: refs/heads/master@{#389030} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
