Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(106)

Issue 87960043: code review 87960043: goprotobuf: Handle nils in repeated message fields in t... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by dsymonds
Modified:
11 years, 6 months ago
Reviewers:
r
CC:
r, golang-codereviews
Visibility:
Public.

Description

goprotobuf: Handle nils in repeated message fields in text formatting. Previously this code would panic. A nil in a repeated message field is, strictly speaking, invalid, but text formatting is often used for debugging, and it's much more useful to generate "<nil>" than to panic.

Patch Set 1 #

Patch Set 2 : diff -r 0bacfa72c808 https://code.google.com/p/goprotobuf #

Patch Set 3 : diff -r 0bacfa72c808 https://code.google.com/p/goprotobuf #

Patch Set 4 : diff -r 0bacfa72c808 https://code.google.com/p/goprotobuf #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -1 line) Patch
M proto/text.go View 1 1 chunk +10 lines, -1 line 0 comments Download
M proto/text_test.go View 1 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 3
dsymonds
Hello r (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/goprotobuf
11 years, 6 months ago (2014-04-15 04:37:16 UTC) #1
r
LGTM
11 years, 6 months ago (2014-04-15 07:16:59 UTC) #2
dsymonds
11 years, 6 months ago (2014-04-15 08:22:28 UTC) #3
*** Submitted as
https://code.google.com/p/goprotobuf/source/detail?r=ec8a922f81f6 ***

goprotobuf: Handle nils in repeated message fields in text formatting.

Previously this code would panic. A nil in a repeated message field is,
strictly speaking, invalid, but text formatting is often used for debugging,
and it's much more useful to generate "<nil>" than to panic.

LGTM=r
R=r
CC=golang-codereviews
https://codereview.appspot.com/87960043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b