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

Jump to bottom

c# review: Inlining tag sizes #518

Closed
jtattermusch opened this issue Jun 19, 2015 · 2 comments
Closed

c# review: Inlining tag sizes #518

jtattermusch opened this issue Jun 19, 2015 · 2 comments
Labels
c#
Milestone
C# code cleanup

Comments

@jtattermusch
Copy link
Contributor

This has already been merged by #515 to csharp-experimental, but would need a second pass review:
jskeet@828b7e6
@jtattermusch
Copy link
Contributor Author

(I think "size += 1 + pb::CodedOutputStream.ComputeRawVarint32Size((uint) dataSize);" is not very understandable because it's not obvious where the "1" comes from, perhaps we could define constants for tag sizes, but let's address that in a follow-up review).

@jtattermusch jtattermusch added this to the C# code cleanup milestone Jun 19, 2015
@jskeet
Copy link
Contributor

jskeet commented Jul 17, 2015

I think defining constants for the tag sizes would make it even more cluttered. Fundamentally, I'm not expecting people to look at the generated code. It shouldn't be ghastly to read, but I'm not bothered by it being slightly obscure. We could certainly document it in a generated code guide though.

adellahlou pushed a commit to adellahlou/protobuf that referenced this issue Apr 20, 2023
@dcodeIO
Added a couple of alternative signatures, protobuf.load returns promi…
e6eaa91 
…se or undefined, aliased Reader/Writer-as-function signature with Reader/Writer.create for typed dialects, see protocolbuffers#518
rinarakaki pushed a commit to rinarakaki/protobuf that referenced this issue Aug 30, 2023
@fowles
Merge pull request protocolbuffers#518 from fowles/main
0a927a7 
Fix missing build deps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c#
Projects
None yet
Milestone
C# code cleanup
Development

No branches or pull requests
2 participants
@jskeet @jtattermusch