Always serialize 64-bit integers as strings in JSON.

Always serialize 64-bit integers as strings in JSON.

This improves compatibility with some other implementations that only
expect strings. Existing implementations using the previous format
should handle both strings and numbers, so switching to pure strings
should be safe.

There is a slight performance impact to this, but only really noticeable
on messages with 100's of repeated int64 fields, so we decided against
adding an option for this.

BUG=https://github.com/dart-lang/dart-protobuf/issues/58
R=skybrian@google.com

Committed: https://github.com/dart-lang/protobuf/commit/221343212db09a35bb9383cb6f3a97c6f3681956

[Jakob Roland Andersen, 2017-01-06 10:41:02]

jakobr@google.com changed reviewers:
+ skybrian@google.com

[skybrian, 2017-01-06 20:17:35]

LGTM. I'm surprised no tests were affected. Maybe write one?

[Jakob Roland Andersen, 2017-01-09 12:25:16]

On 2017/01/06 20:17:35, skybrian wrote:
> LGTM. I'm surprised no tests were affected. Maybe write one?

I had the same thought after sending this one out. Done.

[skybrian, 2017-01-10 07:34:53]

On 2017/01/09 12:25:16, Jakob Roland Andersen wrote:
> On 2017/01/06 20:17:35, skybrian wrote:
> > LGTM. I'm surprised no tests were affected. Maybe write one?
> 
> I had the same thought after sending this one out. Done.

LGTM

Also, you can run the tests in dart-protoc-plugin by temporarily changing the
'protobuf' dependency to a path dependency in in pubspec.yaml.

[skybrian, 2017-01-10 07:37:12]

On 2017/01/10 07:34:53, skybrian wrote:
> On 2017/01/09 12:25:16, Jakob Roland Andersen wrote:
> > On 2017/01/06 20:17:35, skybrian wrote:
> > > LGTM. I'm surprised no tests were affected. Maybe write one?
> > 
> > I had the same thought after sending this one out. Done.
> 
> LGTM
> 
> Also, you can run the tests in dart-protoc-plugin by temporarily changing the
> 'protobuf' dependency to a path dependency in in pubspec.yaml.

It's also a good idea to run the dart-protobuf tests in Chrome:

pub run test -p chrome

[Jakob Roland Andersen, 2017-01-10 08:53:05]

On 2017/01/10 07:37:12, skybrian wrote:
> On 2017/01/10 07:34:53, skybrian wrote:
> > On 2017/01/09 12:25:16, Jakob Roland Andersen wrote:
> > > On 2017/01/06 20:17:35, skybrian wrote:
> > > > LGTM. I'm surprised no tests were affected. Maybe write one?
> > > 
> > > I had the same thought after sending this one out. Done.
> > 
> > LGTM
> > 
> > Also, you can run the tests in dart-protoc-plugin by temporarily changing
the
> > 'protobuf' dependency to a path dependency in in pubspec.yaml.

Ah, so *that's* where the JSON encoding test was. :)

I'll send out a separate review for updating the test there. I'd already put in
a dependency_override to run the benchmarks, but I hadn't thought to run the
tests there, too. We really need to consolidate those two repos, or at least
move the test code to so it's in the same repo as the code it's testing...

> It's also a good idea to run the dart-protobuf tests in Chrome:
> 
> pub run test -p chrome

I ran the dart-protobuf tests with -p "chrome,vm", both passed.

[Jakob Roland Andersen, 2017-01-10 08:53:18]

Description was changed from

==========
Always serialize 64-bit integers as strings in JSON.

This improves compatibility with some other implementations that only
expect strings. Existing implementations using the previous format
should handle both strings and numbers, so switching to pure strings
should be safe.

There is a slight performance impact to this, but only really noticeable
on messages with 100's of repeated int64 fields, so we decided against
adding an option for this.

BUG=https://github.com/dart-lang/dart-protobuf/issues/58
==========

to

==========
Always serialize 64-bit integers as strings in JSON.

This improves compatibility with some other implementations that only
expect strings. Existing implementations using the previous format
should handle both strings and numbers, so switching to pure strings
should be safe.

There is a slight performance impact to this, but only really noticeable
on messages with 100's of repeated int64 fields, so we decided against
adding an option for this.

BUG=https://github.com/dart-lang/dart-protobuf/issues/58
R=skybrian@google.com

Committed:
https://github.com/dart-lang/protobuf/commit/221343212db09a35bb9383cb6f3a97c6...
==========

[Jakob Roland Andersen, 2017-01-10 08:53:19]

Committed patchset #2 (id:20001) manually as
221343212db09a35bb9383cb6f3a97c6f3681956 (presubmit successful).