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

Detect generated code of WKT, addressbook and conformance protos #2822

Merged
merged 1 commit into from
Mar 10, 2017
Merged

Conversation

anandolee
Copy link
Contributor

@anandolee anandolee commented Mar 9, 2017

Add auto detect for generated code of WKT protos, addressbook.proto and conformance.proto

fixes #1212

@bazel-io
Copy link

bazel-io commented Mar 9, 2017

Can one of the admins verify this patch?

Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

How hard would it be to read these parameters from a file instead? That way we could generate that file when we generate the protos with generate_all.sh or whatever it's called.

Just a thought for future expansion, really - it would be very easy to add a proto to be generated but forget to add it here.

@anandolee
Copy link
Contributor Author

I'd prefer only add .proto files that are stable into the detect test. We'd better not add unittest .proto files into it which may both other languages too much. Keep these parameters in hard code makes the adding more serious and formal.

@jskeet
Copy link
Contributor

jskeet commented Mar 10, 2017

So long as people know to make the change, of course. I agree it should be relatively rare. Do we have a document somewhere saying "When you add a WKT or similar, you should change X files; when you modify an existing WKT you should rerun Y generation." ?

@anandolee
Copy link
Contributor Author

anandolee commented Mar 10, 2017

It is mentioned in the comments of this file:

// If this test fails, run the script
// "generate_descriptor_proto.sh" and add the changed files under
// csharp/src/ to your changelist.

And the error message will show which file should be regenerated and add to change list too:

EXPECT_TRUE(actual_contents == *expected_contents)
 << physical_filename << " needs to be regenerated. Please run "
 "generate_descriptor_proto.sh. Then add this file "

Is it enough?

@jskeet
Copy link
Contributor

jskeet commented Mar 10, 2017

That's okay for knowing what to do if the test fails - it's more when the tests should be expanded. But that can definitely come later.

@anandolee anandolee merged commit a69bc9d into protocolbuffers:master Mar 10, 2017
bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
Detect generated code of WKT, addressbook and conformance protos
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.

Detect descriptor.proto changes and fail if C# needs regenerating
4 participants