Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bazel rules for Python grpcio_reflection #16831

Merged
merged 1 commit into from
Oct 12, 2018
Merged

Bazel rules for Python grpcio_reflection #16831

merged 1 commit into from
Oct 12, 2018

Conversation

ghostwriternr
Copy link
Contributor

@ghostwriternr ghostwriternr commented Oct 11, 2018 •
edited
Loading

Add Bazel rules for building and testing grpcio_reflection.

@grpc-testing
Copy link 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link 

[trickle] No significant performance differences

@grpc-testing
Copy link 

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,918      Total (=)      1,965,918

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,891,791      Total (<)     10,891,798

 No significant differences in binary sizes

@grpc-testing
Copy link 

[microbenchmarks] No significant performance differences

nathanielmanistaatgoogle
nathanielmanistaatgoogle approved these changes Oct 11, 2018
View reviewed changes
Copy link
Member

@nathanielmanistaatgoogle nathanielmanistaatgoogle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me.

@mehrdada, @srini100: any concerns from your side?

@grpc-testing
Copy link 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link 

[trickle] No significant performance differences

@grpc-testing
Copy link 

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,965,822      Total (=)      1,965,822

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,909,002      Total (<)     10,909,003

 No significant differences in binary sizes

@grpc-testing
Copy link 

[microbenchmarks] No significant performance differences

@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 73419c4 into grpc:master Oct 12, 2018
@mehrdada
Copy link
Member

mehrdada commented Oct 12, 2018 •
edited
Loading

@nathanielmanistaatgoogle @ghostwriternr This PR is breaking our internal import process, as it consists of rules in BUILD that are incompatible with Blaze. I wonder if they should be moved to BUILD.bazel as opposed to BUILD

@mehrdada mehrdada mentioned this pull request Oct 12, 2018
Merged
@ghostwriternr
Copy link
Contributor Author

@mehrdada I don't know about Blaze, but I've added PR #16859 to rename the BUILD files to BUILD.bazel. Can you please test and check if it helps with the issue?

@nathanielmanistaatgoogle nathanielmanistaatgoogle mentioned this pull request Nov 2, 2018
Merged
@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mehrdada mehrdada mehrdada approved these changes

@nathanielmanistaatgoogle nathanielmanistaatgoogle nathanielmanistaatgoogle approved these changes

@srini100 srini100 Awaiting requested review from srini100

Assignees

@nathanielmanistaatgoogle nathanielmanistaatgoogle

Labels
infra/Bazel lang/Python priority/P2 release notes: no Indicates if PR should not be in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ghostwriternr @grpc-testing @mehrdada @nathanielmanistaatgoogle @kokoro-team