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

Better protbuf version pin #844

Closed
wants to merge 1 commit into from
Closed

Conversation

gen1us2k
Copy link

@gen1us2k gen1us2k commented Jan 8, 2019

No description provided.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@codecov-io
Copy link

Codecov Report

Merging #844 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #844   +/-   ##
=======================================
  Coverage   50.46%   50.46%           
=======================================
  Files          39       39           
  Lines        3761     3761           
=======================================
  Hits         1898     1898           
  Misses       1681     1681           
  Partials      182      182

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae4bb97...b4820be. Read the comment docs.

@gen1us2k
Copy link
Author

gen1us2k commented Jan 8, 2019

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Jan 8, 2019
@johanbrandhorst
Copy link
Collaborator

Hi @gen1us2k. This is unfortunately one of those it's not that simple cases. We're very keen to keep the versions pinned between our Bazel dependencies and our dep dependencies the same, and this PR only updates the dep version. Indeed, the comment just above the line that you edited points to the other file that you'd need to edit. Can you see if the Bazel version can be updated in this fashion too?

The longer term solution to this is #755. If you are interested in implementing that, please do, but this PR as it is is incomplete.

@johanbrandhorst
Copy link
Collaborator

Closing this due to inactivity, please open another one if you wish to continue this work.

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

Successfully merging this pull request may close these issues.

4 participants