-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Accept boxed conditions for run_if, and cleanup IntoScheduleConfigs
#21977
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
base: main
Are you sure you want to change the base?
Conversation
ace3298 to
3d9f97d
Compare
|
Could this also be solved by implementing |
Yea cart has historically wanted to avoid doing that because of potential double/triple-boxing of systems. Note that you can already provide boxed systems via |
Not a big fan of these renames |
I do agree |
pablo-lua
left a comment
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.
Good cleanup, it even simplify some functions there. LGTM
|
This seems technically competent (although I have naming quibbles), but I'm not sold on motivation here and I'm leery of incremental change to scheduling. Blocking per #20115 (comment). This work needs a proper design doc and working group. |
Objective
run_if_dyn#18585We can introduce another layer of traits in order to directly accept boxed conditions as the argument to
IntoScheduleConfigs::run_if.Solution
IntoBoxedConditiontrait which is used to convertIntoSystemconditions andBoxedConditonintoBoxedConditions.ScheduleConfigs::x_innerfunctions toadd_x/set_xand make them public. These are simply the&mut selfversions of the functions onIntoScheduleConfigs, so we should expose them.impl IntoScheduleConfigs for ScheduleConfigssince we now use thoseadd_xfunctions in the default function implementations inIntoScheduleConfigs.Testing
Added a new test for supplying a
BoxedConditiontorun_if.