Skip to content

Rebuild proto artifacts from existing sources and update README.md instructions. #1058

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

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

mikelehen
Copy link
Contributor

This is the result of me updating README.md and following the steps in it. Some of the diffs were for a proto change Greg made a while ago. I'm not really sure why Firestore.pbrpc.h and Firestore.pbrpc.m changed. Everything seems to still build, but if you suspect I did something wrong, let me know. I've never done this before.

I'm doing this in preparation for /actually/ updating the protos.

@wilhuff
Copy link
Contributor

wilhuff commented Apr 11, 2018

The spurious diff you've encountered comes from the fact that the generation strategy for imports changed. It was released with gRPC 1.8.0, which went live 12/12/2017.

I last generated these files before our initial release which was merged into this repo in bde743e on 10/3/2017. I attempted to regenerate this in January in bc74670, but rolled it back after the change didn't work as expected. That change included these diffs.

It's likely that when @rsgowman was working in here on adding nanopb support he just reverted these diffs as being unrelated?

Basically: I don't see anything wrong with this.

@wilhuff wilhuff assigned mikelehen and unassigned wilhuff Apr 11, 2018
@mikelehen mikelehen assigned wilhuff and unassigned mikelehen Apr 11, 2018
@mikelehen
Copy link
Contributor Author

Thanks! If you don't see anything wrong, could you approve it then? :-) Or is there something extra I should do first? [I'll have another PR that actually updates the protos, but it'll be good to get these diffs checked in separately I think...]

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM (doh)

@wilhuff wilhuff assigned mikelehen and unassigned wilhuff Apr 11, 2018
@mikelehen mikelehen merged commit 09e4edd into master Apr 11, 2018
@mikelehen mikelehen deleted the mikelehen/build-protos branch April 11, 2018 15:55
minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request Jun 6, 2018
@firebase firebase locked and limited conversation to collaborators Nov 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants