Skip to content

refactor: prepare for multi-return values in GenerateForAnnotation #240

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

Merged
merged 3 commits into from
Jun 18, 2018

Conversation

kevmoo
Copy link
Collaborator

@kevmoo kevmoo commented Jun 18, 2018

JsonSerializableGenerator now returns String instead of Future
GeneratorHelper is now private and in the same file is the Generator
Encoding and decoding have been split into mixin classes
These two aspects of the generator are nicely isolated hopefully
making future fixes and additions easier

Many members of utils.dart have been moved closer to where they are used
and made private when possible

Updated tests to be sync now that the core tested function is sync

JsonSerializableGenerator now returns String instead of Future<String>
GeneratorHelper is now private and in the same file is the Generator
Encoding and decoding have been split into mixin classes
  These two aspects of the generator are nicely isolated hopefully
  making future fixes and additions easier

Many members of utils.dart have been moved closer to where they are used
  and made private when possible

Updated tests to be sync now that the core tested function is sync
@kevmoo kevmoo requested a review from jakemac53 June 18, 2018 03:33
import 'type_helper.dart';
import 'utils.dart';

abstract class DencodeHelper implements HelperCore {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: DecodeHelper

@@ -0,0 +1,302 @@
import 'package:analyzer/dart/element/element.dart';
Copy link
Collaborator

@jakemac53 jakemac53 Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyright (others also)

Copy link
Collaborator

@jakemac53 jakemac53 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not re-review internals, this is mostly just moves right?

@kevmoo
Copy link
Collaborator Author

kevmoo commented Jun 18, 2018

Did not re-review internals, this is mostly just moves right?

Yup. Mostly copy-paste. The mixin thing is pretty nice...

@jakemac53
Copy link
Collaborator

Ya, I like the new structure more for sure

@kevmoo kevmoo merged commit 02318e0 into master Jun 18, 2018
@kevmoo kevmoo deleted the source_gen_multi_output branch June 18, 2018 15:05
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