Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(32)

Issue 2440763002: Fix Android build issues and gRPC LB policy (Closed)

Created:
2 years, 10 months ago by perumaal
Modified:
2 years, 10 months ago
Reviewers:
xyzzyz, Garrett Casto
CC:
chromium-reviews
Target Ref:
refs/heads/chromium-deps/2016-10-18
Visibility:
Public.

Description

Fix Android build issues and gRPC LB policy * New build.yaml changes and get rid of custom changes. * By default excludes Census LB alone. * Android fixes for logging purposes. * Reduces overall binary/dep size because of the LB changes. R=gcasto@chromium.org, xyzzyz@chromium.org Committed: https://chromium.googlesource.com/external/github.com/grpc/grpc/+/7032ff02c405fc51cfe2a4b8e350547f046ee250

Patch Set 1 #

Patch Set 2 : Fix Android build issues and gRPC LB policy #

Patch Set 3 : Reverting unnecessary files #

Patch Set 4 : Synced to latest #

Total comments: 6

Patch Set 5 : Add self to OWNERS #

Patch Set 6 : Fix minor typo #

Total comments: 1

Patch Set 7 : Remove extra warning #

Total comments: 1

Patch Set 8 : Sync BUILD.gn #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -62 lines) Patch
M BUILD.gn View 1 2 3 4 5 6 7 28 chunks +48 lines, -37 lines 1 comment Download
M OWNERS View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M build.yaml View 1 2 2 chunks +0 lines, -4 lines 0 comments Download
M src/core/plugin_registry/grpc_plugin_registry.c View 1 2 3 chunks +0 lines, -8 lines 0 comments Download
M src/core/plugin_registry/grpc_unsecure_plugin_registry.c View 1 2 3 2 chunks +0 lines, -12 lines 0 comments Download
M templates/BUILD.gn.template View 1 2 3 4 5 6 2 chunks +13 lines, -0 lines 0 comments Download
M test/core/surface/public_headers_must_be_c89.c View 1 2 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 27 (5 generated)
perumaal
2 years, 10 months ago (2016-10-20 17:40:13 UTC) #2
xyzzyz
Can you remove files we don't care about from this commit, like BUILD, CMakeLists.txt etc? ...
2 years, 10 months ago (2016-10-20 18:11:40 UTC) #5
perumaal
PTAL thanks! Removed unnecessary files.
2 years, 10 months ago (2016-10-20 23:58:36 UTC) #6
xyzzyz
https://codereview.chromium.org/2440763002/diff/40001/OWNERS File OWNERS (right): https://codereview.chromium.org/2440763002/diff/40001/OWNERS#newcode4 OWNERS:4: <<<<<<< HEAD Remove merge conflict marks. https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registry/grpc_unsecure_plugin_registry.c File src/core/plugin_registry/grpc_unsecure_plugin_registry.c ...
2 years, 10 months ago (2016-10-21 20:14:39 UTC) #7
Garrett Casto
The comment implies that removing Census from the build means we can carry less local ...
2 years, 10 months ago (2016-10-21 20:18:25 UTC) #8
perumaal
Garrett: build.yaml now generates the two plugin files automatically. There are two more commented out ...
2 years, 10 months ago (2016-10-21 20:23:04 UTC) #9
perumaal
https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registry/grpc_unsecure_plugin_registry.c File src/core/plugin_registry/grpc_unsecure_plugin_registry.c (left): https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registry/grpc_unsecure_plugin_registry.c#oldcode68 src/core/plugin_registry/grpc_unsecure_plugin_registry.c:68: // grpc_register_plugin(grpc_lb_policy_grpclb_init, On 2016/10/21 at 20:14:38, xyzzyz wrote: > ...
2 years, 10 months ago (2016-10-21 20:25:38 UTC) #10
perumaal
> https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.template#newcode31 > templates/BUILD.gn.template:31: "-Wimplicit-function-declaration", > Why do we need -Wimplicit-function-declaration? Oh, I forgot exactly ...
2 years, 10 months ago (2016-10-21 20:26:23 UTC) #11
perumaal
More details on why we don't need to manually edit the plugin files: plugin code ...
2 years, 10 months ago (2016-10-21 20:28:56 UTC) #12
xyzzyz
On 2016/10/21 20:25:38, perumaal wrote: > https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registry/grpc_unsecure_plugin_registry.c > File src/core/plugin_registry/grpc_unsecure_plugin_registry.c (left): > > https://codereview.chromium.org/2440763002/diff/40001/src/core/plugin_registry/grpc_unsecure_plugin_registry.c#oldcode68 > ...
2 years, 10 months ago (2016-10-21 20:40:26 UTC) #13
xyzzyz
On 2016/10/21 20:26:23, perumaal wrote: > > > https://codereview.chromium.org/2440763002/diff/40001/templates/BUILD.gn.template#newcode31 > > templates/BUILD.gn.template:31: "-Wimplicit-function-declaration", > > ...
2 years, 10 months ago (2016-10-21 20:42:12 UTC) #14
xyzzyz
https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.template File templates/BUILD.gn.template (right): https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.template#newcode77 templates/BUILD.gn.template:77: or "census" in f Can we remove these now?
2 years, 10 months ago (2016-10-21 20:42:49 UTC) #15
perumaal
On 2016/10/21 at 20:42:49, xyzzyz wrote: > https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.template > File templates/BUILD.gn.template (right): > > https://codereview.chromium.org/2440763002/diff/80001/templates/BUILD.gn.template#newcode77 ...
2 years, 10 months ago (2016-10-21 20:58:20 UTC) #16
perumaal
On 2016/10/21 at 20:42:12, xyzzyz wrote: > On 2016/10/21 20:26:23, perumaal wrote: > > > ...
2 years, 10 months ago (2016-10-21 21:00:40 UTC) #17
xyzzyz
lgtm https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn#newcode29 BUILD.gn:29: "-Wimplicit-function-declaration", Please regenerate the BUILD.gn file -- you ...
2 years, 10 months ago (2016-10-21 22:41:05 UTC) #18
perumaal
On 2016/10/21 at 22:41:05, xyzzyz wrote: > lgtm > > https://codereview.chromium.org/2440763002/diff/100001/BUILD.gn > File BUILD.gn (right): ...
2 years, 10 months ago (2016-10-21 22:53:49 UTC) #19
perumaal
2 years, 10 months ago (2016-10-21 22:53:51 UTC) #20
Garrett Casto
https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn#newcode539 BUILD.gn:539: "include/grpc/impl/codegen/gpr_types.h", Why are there new includes added after this ...
2 years, 10 months ago (2016-10-21 22:59:32 UTC) #21
perumaal
On 2016/10/21 at 22:59:32, gcasto wrote: > https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn > File BUILD.gn (right): > > https://codereview.chromium.org/2440763002/diff/120001/BUILD.gn#newcode539 ...
2 years, 10 months ago (2016-10-21 23:04:39 UTC) #22
Garrett Casto
lgtm
2 years, 10 months ago (2016-10-21 23:35:09 UTC) #23
perumaal
2 years, 10 months ago (2016-10-21 23:35:34 UTC) #24
perumaal
2 years, 10 months ago (2016-10-22 00:10:31 UTC) #27
Message was sent while issue was closed.
Committed patchset #8 (id:120001) manually as
7032ff02c405fc51cfe2a4b8e350547f046ee250 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698