|
|
DescriptionGet rid of the base_paths target.
Fold the sources back into the base target. There's no good reason to have a
separate target to limit visiblity to :base.
BUG=686208, 706698
Review-Url: https://codereview.chromium.org/2797343002
Cr-Commit-Position: refs/heads/master@{#462265}
Committed: https://chromium.googlesource.com/chromium/src/+/2a567808f0a58c27d84845c8c178f33e79a1426b
Patch Set 1 #Patch Set 2 : add is_linux #
Total comments: 1
Dependent Patchsets: Messages
Total messages: 19 (11 generated)
erikchen@chromium.org changed reviewers: + brettw@chromium.org
brettw: Please review.
The CQ bit was checked by erikchen@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 checked by erikchen@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...
lgtm
The CQ bit was checked by erikchen@chromium.org
The CQ bit was unchecked by erikchen@chromium.org
Can we include BUG=686208,706698 in the CL description, since those were the bugs relating to this?
Description was changed from ========== Get rid of the base_paths target. Fold the sources back into the base target. There's no good reason to have a separate target to limit visiblity to :base. BUG= ========== to ========== Get rid of the base_paths target. Fold the sources back into the base target. There's no good reason to have a separate target to limit visiblity to :base. BUG=686208,706698 ==========
On 2017/04/05 22:09:20, Wez wrote: > Can we include BUG=686208,706698 in the CL description, since those were the > bugs relating to this? done
The CQ bit was checked by erikchen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2797343002/diff/20001/base/BUILD.gn File base/BUILD.gn (right): https://codereview.chromium.org/2797343002/diff/20001/base/BUILD.gn#newcode1622 base/BUILD.gn:1622: allow_circular_includes_from = public_deps As discussed, this GN check hint seems masked the fact that base_paths lacked relevant dependencies - is there a path toward removing it? If not then could GN be educated to build components in allow_circular_includes_from only after all other deps & public_deps have been built, to mitigate this case?
On 2017/04/05 22:18:25, Wez wrote: > https://codereview.chromium.org/2797343002/diff/20001/base/BUILD.gn > File base/BUILD.gn (right): > > https://codereview.chromium.org/2797343002/diff/20001/base/BUILD.gn#newcode1622 > base/BUILD.gn:1622: allow_circular_includes_from = public_deps > As discussed, this GN check hint seems masked the fact that base_paths lacked > relevant dependencies - is there a path toward removing it? > > If not then could GN be educated to build components in > allow_circular_includes_from only after all other deps & public_deps have been > built, to mitigate this case? I was going to follow up with a CL that tries removing it...but I suspect that there will be other violations.
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491430269214530, "parent_rev": "8b717ea665ea3b58b091ee5fe974c14b49bac6f1", "commit_rev": "2a567808f0a58c27d84845c8c178f33e79a1426b"}
Message was sent while issue was closed.
Description was changed from ========== Get rid of the base_paths target. Fold the sources back into the base target. There's no good reason to have a separate target to limit visiblity to :base. BUG=686208,706698 ========== to ========== Get rid of the base_paths target. Fold the sources back into the base target. There's no good reason to have a separate target to limit visiblity to :base. BUG=686208,706698 Review-Url: https://codereview.chromium.org/2797343002 Cr-Commit-Position: refs/heads/master@{#462265} Committed: https://chromium.googlesource.com/chromium/src/+/2a567808f0a58c27d84845c8c178... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2a567808f0a58c27d84845c8c178... |