Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2717493002:
Fix repeated "-Wunknown-pragmas -Wno-error=unknown-pragmas". (Closed)
DescriptionFix repeated "-Wunknown-pragmas -Wno-error=unknown-pragmas".
As gn only deduplicate the configs but not the configs' flags, the
flags "-Wunknown-pragmas -Wno-error=unknown-pragmas" where repeated
for each dependent target using the "grit" template. Instead add a
single config adding those flag and reference it from the "grit"
template to avoid the duplicates.
BUG=None
Review-Url: https://codereview.chromium.org/2717493002
Cr-Commit-Position: refs/heads/master@{#452543}
Committed: https://chromium.googlesource.com/chromium/src/+/f673f4c59c985aa8ddfab2b3821c42af25d915e4
Patch Set 1 #
Total comments: 4
Patch Set 2 : Fix grammar errors. #
Messages
Total messages: 22 (14 generated)
The CQ bit was checked by sdefresne@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: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by sdefresne@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...
Patchset #1 (id:1) has been deleted
sdefresne@chromium.org changed reviewers: + lizeb@chromium.org, scottmg@chromium.org
lizeb: can you take a look? scottmg: can you take a look as OWNERS? For example, when building chrome/browser/browser/resource_prefetcher.cc for Android, the flags were repeated 31 times on the command-line.
On 2017/02/23 14:32:03, sdefresne wrote: > lizeb: can you take a look? > scottmg: can you take a look as OWNERS? > > For example, when building chrome/browser/browser/resource_prefetcher.cc for > Android, the flags were repeated 31 times on the command-line. \o/ 1. Start trolling 2. ... 3. sdefresne@ puts out a useful CL I don't know anything about gn, so I defer to scottmg.
https://codereview.chromium.org/2717493002/diff/20001/build/config/compiler/B... File build/config/compiler/BUILD.gn (right): https://codereview.chromium.org/2717493002/diff/20001/build/config/compiler/B... build/config/compiler/BUILD.gn:1696: # but only the config themselves, if the template declare a "config" for that nit: declares https://codereview.chromium.org/2717493002/diff/20001/build/config/compiler/B... build/config/compiler/BUILD.gn:1698: # Using a separate config solve this problem. nit: solves
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thank you for the reviews. https://codereview.chromium.org/2717493002/diff/20001/build/config/compiler/B... File build/config/compiler/BUILD.gn (right): https://codereview.chromium.org/2717493002/diff/20001/build/config/compiler/B... build/config/compiler/BUILD.gn:1696: # but only the config themselves, if the template declare a "config" for that On 2017/02/23 14:59:22, Benoit L wrote: > nit: declares Done. https://codereview.chromium.org/2717493002/diff/20001/build/config/compiler/B... build/config/compiler/BUILD.gn:1698: # Using a separate config solve this problem. On 2017/02/23 14:59:22, Benoit L wrote: > nit: solves Done.
The CQ bit was checked by sdefresne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org Link to the patchset: https://codereview.chromium.org/2717493002/#ps40001 (title: "Fix grammar errors.")
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": 40001, "attempt_start_ts": 1487868884751380,
"parent_rev": "fd848ca355cc7b9034a7883f50ad4f9f5dec1b91", "commit_rev":
"f673f4c59c985aa8ddfab2b3821c42af25d915e4"}
Message was sent while issue was closed.
Description was changed from ========== Fix repeated "-Wunknown-pragmas -Wno-error=unknown-pragmas". As gn only deduplicate the configs but not the configs' flags, the flags "-Wunknown-pragmas -Wno-error=unknown-pragmas" where repeated for each dependent target using the "grit" template. Instead add a single config adding those flag and reference it from the "grit" template to avoid the duplicates. BUG=None ========== to ========== Fix repeated "-Wunknown-pragmas -Wno-error=unknown-pragmas". As gn only deduplicate the configs but not the configs' flags, the flags "-Wunknown-pragmas -Wno-error=unknown-pragmas" where repeated for each dependent target using the "grit" template. Instead add a single config adding those flag and reference it from the "grit" template to avoid the duplicates. BUG=None Review-Url: https://codereview.chromium.org/2717493002 Cr-Commit-Position: refs/heads/master@{#452543} Committed: https://chromium.googlesource.com/chromium/src/+/f673f4c59c985aa8ddfab2b3821c... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/f673f4c59c985aa8ddfab2b3821c...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:40001) has been created in https://codereview.chromium.org/2708923007/ by prasadv@chromium.org. The reason for reverting is: Looks like this CL is breaking Android Compile on chromium.perf. https://bugs.chromium.org/p/chromium/issues/detail?id=695534. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
