-
Notifications
You must be signed in to change notification settings - Fork 680
RawModuleDefV10 Scheduled functions should not be callable
#4179
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: master
Are you sure you want to change the base?
Conversation
5204010 to
1d30ed6
Compare
1d30ed6 to
583b7e8
Compare
RawModuleDef V10 Scheduled functions should not be callableRawModuleDefV10 Scheduled functions should not be callable
|
Wait, doesn't the module owner still need the option/ability to call private reducers? |
Well, if that’s the case, then I misunderstood the requirement. I thought they were only meant to be called by the Host. |
|
Re: making internals callable by the owner, I would suggest that:
|
This would break BitCraft's workflow as they sometimes need to be able to call scheduled reducers manually. I believe that wholly disallowing is a regression. Note for example that the roadmap item says "Make scheduled reducers internal by default" (emphasis mine). It is not that scheduled reducers are only callable by the module. I think this ticket was also just underspecified in general. I would add the following requirements:
#[reducer(public)]
fn foobar() {
// ...
} |
It was my understanding that we would, in fact, do exactly your 4 here, we just didn't need to do so in the MVP. |
|
Ok @Shubham8287 , per discussion with @cloutiertyler out-of-band, please:
|
Description of Changes
visibility == FunctionVisiblity::InternalUpdateHostto check for visibility before excuting reducers and procedures.API and ABI breaking changes
NA, master is not exposing new raw module version yet.
Expected complexity level and risk
Testing
Smoketest can be in
2.0-breaking-changesbranch once we merge this.Exising test should cover for regression