Skip to content

Fix detection of toJson/fromJson in nested types #193

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 28, 2018
Merged

Fix detection of toJson/fromJson in nested types #193

merged 1 commit into from
May 28, 2018

Conversation

kevmoo
Copy link
Collaborator

@kevmoo kevmoo commented May 28, 2018

No description provided.

@kevmoo kevmoo requested review from jakemac53 and natebosch May 28, 2018 01:15
natebosch
natebosch reviewed May 28, 2018
View reviewed changes
Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

Please expand the commit message with a description of the changes

json_serializable/lib/src/type_helpers/json_helper.dart Outdated
return false;
}

JsonSerializable _getAnnotation(InterfaceType source) {
Copy link
Member

Choose a reason for hiding this comment

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

[nit] how about annotation.

https://www.dartlang.org/guides/language/effective-dart/design#avoid-starting-a-method-name-with-get

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@kevmoo
Fix detection of toJson/fromJson in nested types
58549c6 
Before: just check for `fromJson` ctor to generate either to or fromJson

Now:
fromJson: check for fromJson ctor or if JsonSerializable.createFactory
  is true
toJson: check for toJson methor or if JsonSerializable.createToJson
  is true
@kevmoo kevmoo force-pushed the better_to_from_json_decetion branch from 6645c79 to 58549c6 Compare May 28, 2018 02:28
@kevmoo
Copy link
Collaborator Author

kevmoo commented May 28, 2018

Updated commit message and rebased. PTAL @natebosch

natebosch
natebosch approved these changes May 28, 2018
View reviewed changes
Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

I think you could add more detail on the commit message if you wanted to 😉

@kevmoo kevmoo merged commit 537933f into master May 28, 2018
@kevmoo kevmoo deleted the better_to_from_json_decetion branch May 28, 2018 17:44
@Gopinathp Gopinathp mentioned this pull request Jan 14, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natebosch natebosch natebosch approved these changes

@jakemac53 jakemac53 Awaiting requested review from jakemac53

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kevmoo @natebosch