-
Notifications
You must be signed in to change notification settings - Fork 165
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
Refresh Callback (+ Extra for AboutToRun) #1884
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I have any additional notes provided you can respond to Al's questions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the feedback.
There was a lot of nice clean up in this PR.
I'm still not completely happy with the OnNextRefresh
naming, as it is more of a OnNextNonRefresh
but it good enough for now.
The goal of the new callback is to fix issues plugin developers have when doing stuff in
onFinishedRun
(or whatever the name is). Now they should be able to wrap what they do withonNextRefresh
and hopefully avoid these problems.This is to be tested, which is why it's WIP.