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

RecordData errors #6197

Merged
merged 1 commit into from
Jun 29, 2019
Merged

RecordData errors #6197

merged 1 commit into from
Jun 29, 2019

Conversation

igorT
Copy link
Member

@igorT igorT commented Jun 26, 2019

Implements emberjs/rfcs#465

Also cleans up some of the feature flagging infra thanks to @rwjblue

@igorT igorT force-pushed the igor/record-data-errors branch 2 times, most recently from caa5172 to 4dadc81 Compare June 26, 2019 23:44
@igorT
Copy link
Member Author

igorT commented Jun 27, 2019

@mike-north reviewed in person

Copy link
Contributor

@runspired runspired left a comment

Choose a reason for hiding this comment

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

Outside of these review comments I have the additional concern that we seem to have put a lot of new private methods on Model. If this logic truly belongs living with the record can it be encapsulated as util functions within the module instead of leaking methods to consumers?

@igorT
Copy link
Member Author

igorT commented Jun 28, 2019

module

Added two methods, which after we have a subscription mechanism with custom model classes can be isolated in a callback/not exposed

@igorT igorT force-pushed the igor/record-data-errors branch 3 times, most recently from 0182979 to 73c1661 Compare June 29, 2019 05:02
@igorT igorT force-pushed the igor/record-data-errors branch from 73c1661 to d011ece Compare June 29, 2019 05:09
@igorT igorT merged commit 2c8efbc into master Jun 29, 2019
@igorT igorT deleted the igor/record-data-errors branch June 29, 2019 05:32
pete-the-pete pushed a commit to pete-the-pete/data that referenced this pull request Jul 4, 2019
pete-the-pete pushed a commit to pete-the-pete/data that referenced this pull request Jul 23, 2019
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.

3 participants