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

Simplify syntax in example code #184

Closed
wants to merge 1 commit into from

Conversation

waldyrious
Copy link
Contributor

@waldyrious
Copy link
Contributor Author

For the record, the discrepancies led to some doubts about what syntax would be valid / recommended. From a brief search it looks like others also had the same doubts.

@chalin chalin self-requested a review April 16, 2020 20:04
@chalin
Copy link
Collaborator

chalin commented Apr 16, 2020

To be fair, there isn't an explicit mention of this in the style guide. But thanks for all the links, including to SO.

I'd rather not update old blog entries. For the rest, I'd rather that the site reflect the actual sources that they are originating from. So this would mean making changes to each of the corresponding repos, and then only making the change here once the original sources have been updated.

I'd suggest that we take it one language at a time. Let me first try with the new Kotlin example, since I've done some cleanup in that .proto file already. I'll report back on results here.

@chalin
Copy link
Collaborator

chalin commented Apr 16, 2020

FYI, cleanup done for gRPC-Kotlin examples - grpc/grpc-kotlin#62.

@waldyrious
Copy link
Contributor Author

waldyrious commented Apr 17, 2020

Makes sense, I'll revert the changes to the blog post.

As for the source repositories, point taken, and thanks for the Kotlin example. Are you planning to make the remaining changes yourself, or should I go ahead and submit PRs?

@chalin
Copy link
Collaborator

chalin commented Apr 17, 2020

Are you planning to make the remaining changes yourself, ...

I'd like to propose simplifications to the .proto files for the two other languages I'm currently working on. Let's see how those are accepted first before tackling all other languages. Thanks.

@chalin
Copy link
Collaborator

chalin commented Jun 11, 2020

Hi @waldyrious: it'll take a while before this is addressed (in part because we need to resolve issues like grpc/grpc-proto#81 first). Can you create an issue with the opening PR comment, and then close this PR? If you'd rather that I do it, let me know.

@chalin chalin added the blocked label Jun 11, 2020
@waldyrious
Copy link
Contributor Author

Thanks for the update. I understand that coordinating changes across several repos is not straightforward.

Can you create an issue with the opening PR comment, and then close this PR? If you'd rather that I do it, let me know.

I'd be more comfortable with you doing that, yeah — your context is more complete than mine.

@chalin
Copy link
Collaborator

chalin commented Jun 12, 2020

Closing. Will track progress via #282.

@chalin chalin closed this Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants