-
Notifications
You must be signed in to change notification settings - Fork 50
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
Empty definition from google #750
Conversation
Instead of using our own, use the google one in the predefined repository.
20b3109
to
ba6e0f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Looks like we still got a few spots that need updating otherwise 👍
ba6e0f4
to
eb0da8f
Compare
It’s all green now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's worth it though... We are adding a new dependency to some packages in order to save a couple of lines in our proto file and it looks strange to have one of our service types separated in another place, no? This feels like "violating" one of the "goverbs" (a little copy is better than a little dependency)
That’s a valid point, worth discussing I guess. |
After a good night of thoughts, I think you are right. This has more drawbacks than advantages. Well, at least, that allowed me to fix the generation action to take latest protoc available rather than the one in the current distro. Let’s withdraw. |
Instead of using our own, use the empty google message in the predefined repository.