Proto3 map support for C# #543
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
More tests required. Generated code in next commit.
Generated code in next commit.
(Primarily this is starting the hash code of messages at a non-zero value...)
Merged
- Change the default message hash code to 1 to be consistent with other code - Change the empty list/map hash code to 0 as "empty map" is equivalent to "no map" - Removed map fields from unittest_proto3.proto - Created map_unittest_proto3.proto which is like map_unittest.proto but proto3-only - Fixed factory methods in FieldCodec highlighted by using all field types :) - Added tests for map serialization: - Extra fields within entries - Entries with value then key - Non-contiguous entries for the same map - Multiple entries for the same key Changes to generated code coming in next commit
|
I've made the requested changes from #544 - I've added all the relevant tests I can think of, and the coverage looks pretty good... are there any other tests you'd like before I merge this? (I haven't rebased the cleanup work yet - I'll do that when I merge this.) |
|
Just found another bug with FieldCodec - will write significant tests for this before merging... |
|
Okay, with the fix to FieldCodec (and some tests for it, finally :) I think I'm happy with this again. |
|
I went through the code and LGTM (ex-post, because it's already been merged). |
Closed
taoso
pushed a commit
to taoso/protobuf
that referenced
this pull request
Aug 1, 2018
) Rework the handling of imported packages. Do not add imported protobuf packages to a global registry. Instead, maintain a per-file list of packages and names. This avoids imports in one file affecting the generated code in a second one. Consistently deal with imports in terms of import path, rather than source file. This avoids imports of two .proto files in the same Go package producing redundant import statements. Introduce ImportPath and PackageName types to prevent confusion about which values contain import paths ("github.com/golang/protobuf/proto") and which contain package names ("proto"). Convert many uses of FileDescriptorProto to FileDescriptor, for consistency and general convenience.
rinarakaki
pushed a commit
to rinarakaki/protobuf
that referenced
this pull request
Aug 30, 2023
Fixes for google3 (layering check and formatting).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are still some open questions about handling strings and ByteStrings, but this is a good first step.