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

Remove the C#-specific field_presence_test.proto, using unittest_no_field_presence.proto instead. #379

Merged
merged 2 commits into from May 13, 2015

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented May 13, 2015

This is the start of establishing a C# namespace of "Google.ProtocolBuffers.TestProtos.Proto3" for proto3-syntax protos.
We could optionally split the directory structure as well into Proto2 and Proto3 for clarity.

This addresses part of issue #374.

…ield_presence.proto instead.

This is the start of establishing a C# namespace of "Google.ProtocolBuffers.TestProtos.Proto3" for proto3-syntax protos.
We could optionally split the directory structure as well into Proto2 and Proto3 for clarity.
@jskeet
Copy link
Contributor Author

jskeet commented May 13, 2015

@jtattermusch This somewhat undoes an earlier PR of yours
@anandolee This removes the "extra" proto you added, so we can use the common one instead

I don't think this will be controversial, but I could be wrong.
Any comments on the use of a ....Proto3 namespace for differentiation? I haven't currently got a Proto2 namespace, but we could do that separately if we wanted to.

@jtattermusch
Copy link
Contributor

It doesn't look like it's undoing anything.

Can you also update the generate_protos.sh script so that it's up to date? After running it, all the code should remain the same with the exception of files that have been manually modified:
modified: csharp/src/ProtocolBuffers.Test/TestProtos/Unittest.cs
modified: csharp/src/ProtocolBuffers.Test/TestProtos/UnittestCustomOptions.cs
modified: csharp/src/ProtocolBuffersLite.Test/TestProtos/Unittest.cs
modified: csharp/src/ProtocolBuffersLite.Test/TestProtos/UnittestLite.cs

@jskeet
Copy link
Contributor Author

jskeet commented May 13, 2015

It's undoing the earlier removal of UnittestNoFieldPresence.cs :)

And yes, I'll fix up the shell script - I meant to do that before.

@jskeet
Copy link
Contributor Author

jskeet commented May 13, 2015

Have added the change to the shell script, but I can't easily test it here - could you pull and check that when you run the shell script, there aren't any further changes?

@jtattermusch
Copy link
Contributor

Just ran generate_protos.sh and all seems good. LGTM.

@jtattermusch
Copy link
Contributor

Jie, please merge once you're happy.

anandolee added a commit that referenced this pull request May 13, 2015
Remove the C#-specific field_presence_test.proto, using unittest_no_field_presence.proto instead.
@anandolee anandolee merged commit 3bc162a into protocolbuffers:csharp May 13, 2015
taoso pushed a commit to taoso/protobuf that referenced this pull request Aug 1, 2018
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.

None yet

4 participants