Add tests for serializer

[ekasper, 2012-08-10 13:12:49]

Ping (in case you missed this one...)

On Wed, Aug 8, 2012 at 5:47 PM, <ekasper@google.com> wrote:

> Reviewers: Al Cutter,
>
>
>
> Please review this at
http://codereview.appspot.com/**6449111/<http://codereview.appspot.com/6449111/>
>
> Affected files:
>   M .hgignore
>   M src/proto/Makefile
>   A src/proto/serializer_test.cc
>
>
>

[Al Cutter, 2012-08-10 15:14:10]

LGTM, couple of nits below

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc
File src/proto/serializer_test.cc (right):

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
src/proto/serializer_test.cc:63: #define S(t, n) std::string((t), (2 * n))
You could just make these functions in the anonymous namespace and avoid having
to undef them later on.

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
src/proto/serializer_test.cc:136: bstring token = B(kDefaultSCHTokenHexString,
kDefaultSCHTokenLength);
indent

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
src/proto/serializer_test.cc:145: bstring token = B(kDefaultSCHTokenHexString,
kDefaultSCHTokenLength);
indent

[ekasper, 2012-08-10 15:43:09]

Thanks, pushing.

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc
File src/proto/serializer_test.cc (right):

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
src/proto/serializer_test.cc:63: #define S(t, n) std::string((t), (2 * n))
On 2012/08/10 15:14:10, Al Cutter wrote:
> You could just make these functions in the anonymous namespace and avoid
having
> to undef them later on.

Done.

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
src/proto/serializer_test.cc:136: bstring token = B(kDefaultSCHTokenHexString,
kDefaultSCHTokenLength);
On 2012/08/10 15:14:10, Al Cutter wrote:
> indent

Done.

http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
src/proto/serializer_test.cc:145: bstring token = B(kDefaultSCHTokenHexString,
kDefaultSCHTokenLength);
On 2012/08/10 15:14:10, Al Cutter wrote:
> indent

Done.

[Ben Laurie (Google), 2012-08-13 12:34:06]

On 10 August 2012 16:14,  <alcutter@google.com> wrote:
> LGTM, couple of nits below
>
>
> http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc
> File src/proto/serializer_test.cc (right):
>
>
http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
> src/proto/serializer_test.cc:63: #define S(t, n) std::string((t), (2 *
> n))
> You could just make these functions in the anonymous namespace and avoid
> having to undef them later on.

What is this obsession with the anonymous namespace? What have you got
against static?

>
>
http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
> src/proto/serializer_test.cc:136: bstring token =
> B(kDefaultSCHTokenHexString, kDefaultSCHTokenLength);
> indent
>
>
http://codereview.appspot.com/6449111/diff/1/src/proto/serializer_test.cc#new...
> src/proto/serializer_test.cc:145: bstring token =
> B(kDefaultSCHTokenHexString, kDefaultSCHTokenLength);
> indent
>
> http://codereview.appspot.com/6449111/