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

php_protobuf_plugin: update checking reserved variables with protobuf 3.5.0 #13580

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

ZhouyihaiDing
Copy link
Contributor

For the issue #13285
Currently, protobuf is bumped to 3.5.0. There are more reserved variables in this version and php plugin can use the new API provided to check reserved variables.

@@ -21,6 +21,7 @@
#include "src/compiler/config.h"
#include "src/compiler/generator_helpers.h"
#include "src/compiler/php_generator_helpers.h"
#include <google/protobuf/compiler/php/php_generator.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure it is better to leave it here(only this file use it), or move it to config.h to make it looks like grpc::protobuf::compiler::php

#include "src/compiler/config.h"
#include "src/compiler/generator_helpers.h"
#include "src/compiler/php_generator_helpers.h"

using google::protobuf::compiler::php::GeneratedClassName;
Copy link
Contributor Author

@ZhouyihaiDing ZhouyihaiDing Dec 11, 2017

Choose a reason for hiding this comment

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

Hi, @jtattermusch . Can you please give me an advise here? Do I need to move include #include <google/protobuf/compiler/php/php_generator.h> to config.h and make it grpc::protobuf::compiler::php?

@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@grpc grpc deleted a comment from grpc-testing Mar 5, 2018
@ZhouyihaiDing
Copy link
Contributor Author

Bazel Debug build for C/C++ and Bazel Opt build for C/C++ look like infrastructure problems in new tests.

@grpc-testing
Copy link

[microbenchmarks] No significant performance differences

@ZhouyihaiDing
Copy link
Contributor Author

ZhouyihaiDing commented Mar 5, 2018

Known issues:
Basic Tests MacOS [dbg] (internal CI) - #14596
Basic Tests Multi-language Linux (internal CI) - #14596
Basic Tests MacOS [opt] (internal CI) - #14596

@ZhouyihaiDing ZhouyihaiDing merged commit d8feec2 into grpc:master Mar 5, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants