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.

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

automatically export //java:grpc_compiletime_deps when with_Grpc = true #55

Merged
merged 2 commits into from
Jan 22, 2017
Merged

automatically export //java:grpc_compiletime_deps when with_Grpc = true #55

merged 2 commits into from
Jan 22, 2017

Conversation

perezd
Copy link
Contributor

@perezd perezd commented Jan 20, 2017

This makes it easier on users to not have to remember to import //java:grpc_compiletime_deps on their own.

@perezd
Copy link
Contributor Author

perezd commented Jan 20, 2017

This addresses #52.

@pcj
Copy link
Contributor

pcj commented Jan 21, 2017

The tests have been a little flaky lately, and travis queue seems to be getting longer and longer. I restarted the job.

@pcj
Copy link
Contributor

pcj commented Jan 21, 2017

Can you:

  1. Document this behavior in the README.md for the java rules?

  2. Comment out the grpc_compiletime_deps in the client and server helloworld example, leaving a comment that it is automatically exported by the proto rule?

@perezd
Copy link
Contributor Author

perezd commented Jan 21, 2017

OK, updated those items.

@perezd
Copy link
Contributor Author

perezd commented Jan 21, 2017

Oh shoot, forgot the comment in the build file...should I add that still?

@pcj
Copy link
Contributor

pcj commented Jan 22, 2017

Might be nice in case someone has a non-first order dependency that won't be satisfied by export, less digging to figure out the magic.

If you want to just commit --amend and git push --force auto-export-grpc won't make an extra commit.

@perezd
Copy link
Contributor Author

perezd commented Jan 22, 2017

OK all patched up.

@pcj pcj merged commit 0e14a81 into pubref:master Jan 22, 2017
@pcj
Copy link
Contributor

pcj commented Jan 22, 2017

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants