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

Add JsonObject class and serializer. #146

Merged
merged 1 commit into from
Mar 10, 2017

Conversation

davidmorgan
Copy link
Collaborator

@davidmorgan davidmorgan commented Mar 9, 2017

Fixes #134

This change is Reviewable

@manuelF
Copy link

manuelF commented Mar 10, 2017

Review status: 0 of 19 files reviewed at latest revision, 2 unresolved discussions.


example/lib/example.dart, line 89 at r2 (raw file):

  final value = standardSerializers.deserializeWith(
      Account.serializer, serializedAccount);
  print(value);

Print in a test?


example/lib/serializers.g.dart, line 56 at r2 (raw file):

      ..add(Account.serializer)
      ..addBuilderFactory(
          const FullType(BuiltMap,

Move it with the rest of the builderfactories?


Comments from Reviewable

@davidmorgan
Copy link
Collaborator Author

Review status: 0 of 19 files reviewed at latest revision, 2 unresolved discussions.


example/lib/example.dart, line 89 at r2 (raw file):

Previously, manuelF (Manuel Ferreria) wrote…

Print in a test?

This is an example, not a test :) ... the existing example also prints things.

(Although we do run the example in a test, to make sure it doesn't crash.)


example/lib/serializers.g.dart, line 56 at r2 (raw file):

Previously, manuelF (Manuel Ferreria) wrote…

Move it with the rest of the builderfactories?

This file is generated :)


Comments from Reviewable

@manuelF
Copy link

manuelF commented Mar 10, 2017

:lgtm:


Review status: 0 of 19 files reviewed at latest revision, 2 unresolved discussions.


example/lib/serializers.g.dart, line 56 at r2 (raw file):

Previously, davidmorgan (David Morgan) wrote…

This file is generated :)

OK, Understood now that it is now depended on so it instantiates it now, not eagerly


Comments from Reviewable

@davidmorgan davidmorgan merged commit 2377508 into google:master Mar 10, 2017
@davidmorgan davidmorgan deleted the json-object branch March 10, 2017 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants