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.

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

[SEMVER-MAJOR] Remove model events #965

Merged
merged 1 commit into from
Jun 8, 2016
Merged

[SEMVER-MAJOR] Remove model events #965

merged 1 commit into from
Jun 8, 2016

Conversation

0candy
Copy link
Contributor

@0candy 0candy commented Jun 6, 2016 •
edited
Loading

@0candy 0candy added the #review label Jun 6, 2016
bajtos
bajtos reviewed Jun 7, 2016
View reviewed changes
3.0-RELEASE-NOTES.md
Model events
*changed
*deletedAll
*deleted
Copy link
Member

Choose a reason for hiding this comment

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

The text are formatted in italics, I think you need to put a space after the star character.

Copy link
Member

Choose a reason for hiding this comment

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

Also please add instructions explaining how to upgrade code depending on these removed events (i.e. use Operation Hooks).

@bajtos
Copy link
Member

bajtos commented Jun 7, 2016

@0candy I think you missed one more instance - lib/dao.js#L585-L587:

$ grep Model.emit -rn lib
lib/dao.js:586:                Model.emit('changed', inst);
lib/dao.js:2588:  Model.emit('set', inst);

I am not sure what's the purpose of the set event and I don't see any alternative how to receive that notification, so let's keep set around.

@bajtos bajtos assigned 0candy and unassigned bajtos Jun 7, 2016
@0candy 0candy assigned bajtos and unassigned 0candy Jun 7, 2016
@bajtos bajtos changed the title Remove model events [SEMVER-MAJOR] Remove model events Jun 7, 2016
@bajtos
Copy link
Member

bajtos commented Jun 7, 2016

LGTM. Please squash the commits before landing.

@bajtos bajtos assigned 0candy and unassigned bajtos Jun 7, 2016
@0candy 0candy merged commit a55bf40 into master Jun 8, 2016
@0candy 0candy deleted the remove_events branch June 8, 2016 15:11
@0candy 0candy removed the #review label Jun 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@0candy 0candy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0candy @bajtos