Skip to content

Introducing AfterConvertCallback/Event #1060

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
Closed
wants to merge 2 commits into from
Closed

Introducing AfterConvertCallback/Event #1060

wants to merge 2 commits into from

Conversation

schauder
Copy link
Contributor

This is to replace the AfterLoadCallback/Event in order to match the naming of other store modules.

AfterLoadCallback/Event is still in place for now, but deprecated.

Closes #1053

@schauder
1053-rename-after-load-to-after-convert - Prepare branch
fd6e437
@schauder
Introducing AfterConvertCallback/Event.
734e0af 
This is to replace the AfterLoadCallback/Event in order to match the naming of other store modules.

AfterLoadCallback/Event is still in place for now, but deprecated.

Closes #1053
@schauder schauder requested a review from mp911de September 27, 2021 08:44
@mp911de
Copy link
Member

mp911de commented Sep 30, 2021

How about deprecating AfterLoadCallback and AfterLoadEvent and letting them subclass from AfterConvertCallback respective AfterConvertEvent?

@schauder
Copy link
Contributor Author

schauder commented Oct 1, 2021

How about deprecating AfterLoadCallback and AfterLoadEvent and letting them subclass from AfterConvertCallback respective AfterConvertEvent?

AfterLoadCallback would then have to public methods expecting an entity, which then would crash the EntityCallbackDiscoverer.lookupCallbackMethod.

Also I'm not sure this would offer much of a benefit, since in the end, users need to change their classes anyway.

mp911de pushed a commit that referenced this pull request Oct 1, 2021
@schauder @mp911de
Introducing AfterConvertCallback/Event.
3cd25ee 
This is to replace the AfterLoadCallback/Event in order to match the naming of other store modules.

AfterLoadCallback/Event is still in place for now, but deprecated.

Closes #1053
Original pull request: #1060.
mp911de added a commit that referenced this pull request Oct 1, 2021
@mp911de
Polishing.
bc866e3 
Add missing deprecation annotation. Fix since tags.

See #1053
Original pull request: #1060.
@mp911de
Copy link
Member

mp911de commented Oct 1, 2021

That's merged and polished now.

@mp911de mp911de closed this Oct 1, 2021
@mp911de mp911de deleted the issue/1053-rename-after-load-to-after-convert branch October 1, 2021 08:34
@mp911de mp911de added this to the 2.3 RC1 (2021.1.0) milestone Oct 1, 2021
@mp911de mp911de added the type: enhancement A general enhancement label Oct 1, 2021
@mp911de mp911de changed the title Introducing AfterConvertCallback/Event. Introducing AfterConvertCallback/Event Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mp911de mp911de Awaiting requested review from mp911de

Assignees

@mp911de mp911de

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
2.3 RC1 (2021.1.0)
Development

Successfully merging this pull request may close these issues.

Replace AfterLoadEvent/Callback with AfterConvertEvent/Callback
2 participants
@schauder @mp911de