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

First steps to making the C# runtime work with new codegen #313

Merged
merged 3 commits into from Apr 28, 2015

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Apr 28, 2015

After committing, the runtime itself should build and tests should pass. The protogen library will no longer build; that is to be removed in the next set of commits.

The new generated files committed here are built with protoc.exe with the new codegen, but with a few modifications.

1) Remove CSharpOptions
2) A new version of DescriptorProtoFile (with manual changes from codegen - it would otherwise be Descriptor.cs)
3) Turn off CLS compliance (which we'll remove from the codebase entirely; I don't think it's actually relevant these days)
4) Add "public imports" to FileDescriptor, with code broadly copied from the Java codebase.
Lots more changes to commit before it will build and tests run, but one step at a time...
This commit includes changes to the C#-specific protos, and rebuilt versions of the "stock" protos.
The stock protos have been locally updated to have a specific C# namespace, but this is expected to change soon, so hasn't been committed.
Four areas are currently not tested:
1) Serialization - we may restore this at some point, possibly optionally.
2) Services - currently nothing is generated for this; will need to see how it interacts with GRPC
3) Fields beginning with _{digit} - see protocolbuffers#308
4) Fields with names which conflict with the declaring type in nasty ways - see protocolbuffers#309
@jskeet
Copy link
Contributor Author

jskeet commented Apr 28, 2015

@jtattermusch to review this - although it's primarily replacing generated code.

@jtattermusch
Copy link
Contributor

Besides the minor comment, LGTM.

Maybe one thing: can you put a list of manual changes you've done into an initial comment in auto generated files?

@jskeet
Copy link
Contributor Author

jskeet commented Apr 28, 2015

Sure - will rebuild, see what the differences are, and then add the comments into a new commit. I've added another commit on my local branch though, so it may involve some shenanigans...

…ges.

(Having regenerated descriptor.proto relative to src, the earlier commented-out code checking that dependencies match may now be okay to uncomment again. Will experiment in later CLs.)
@jskeet
Copy link
Contributor Author

jskeet commented Apr 28, 2015

Added commit 5ca6dd7 to indicate the manual changes. It looks like it's changing more because it was generated by running protoc with just -I src instead of -I src\google\protobuf; the new file is the more appropriate one.

@jskeet
Copy link
Contributor Author

jskeet commented Apr 28, 2015

@anandolee This is now ready to merge if you're happy.

@anandolee anandolee added the c# label Apr 28, 2015
anandolee added a commit that referenced this pull request Apr 28, 2015
First steps to making the C# runtime work with new codegen
@anandolee anandolee merged commit 32ead75 into protocolbuffers:csharp Apr 28, 2015
taoso pushed a commit to taoso/protobuf that referenced this pull request Aug 1, 2018
Found with honnef.co/go/tools/cmd/unused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants