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

Defer until after perform instead of all_perform #366

Conversation

mjonas87
Copy link
Contributor

@mjonas87 mjonas87 commented Sep 26, 2023

Description

all_perform is a redundant set of callbacks and is entirely enveloped by the Organizer's perform callbacks. This change moves the execution of the deferred callbacks to after perform rather than all_perform to better match the intent of the defer_after_callbacks_when_organized attribute.

The impact of this should be very minimal as, in both cases, the callbacks are still deferred until after all nested interactors have finished.

Information

  • [ ] Contains Documentation It's a bug fix for logic that is already documented.
  • [ ] Contains Tests Left tests unmodified to validate it is not a breaking change
  • [ ] Contains Breaking Changes Does not.

Changelog

Fixed

  • Fixes a minor timing issue for when deferred after_perform callbacks are run.

all_perform was a redundant callbacks and is entirely enveloped by the Organizer's `perform` callbacks. This change moves the execution of the deferred callbacks to after `perform` rather than `all_perform` to better match the intent of the `defer_after_callbacks_when_organized` attribute.
@mjonas87
Copy link
Contributor Author

mjonas87 commented Sep 26, 2023

@aaronmallen This fix was bundled up in the breaking change I submitted previously. Pulled out the breaking change.

@aaronmallen aaronmallen merged commit 040dea0 into aaronmallen:main Sep 30, 2023
8 checks passed
@mjonas87
Copy link
Contributor Author

Thank you!

@mjonas87 mjonas87 deleted the run_deferred_callbacks_after_organizer_perform branch October 1, 2023 02:30
@aaronmallen aaronmallen mentioned this pull request Oct 1, 2023
3 tasks
@aaronmallen
Copy link
Owner

Thanks for your contribution, this has been released in v1.2.2

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