Fix non-functional nondeterministic_system_order example #10719
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
OK, that red mark looks like unrelated trailing whitespace in the cargo.toml. Happy to fix it in this PR, but only if someone asks me to, since I assume it's going to jump out at everyone else's PRs too. |
Kanabenki
added
C-Bug
alice-i-cecile
approved these changes
Nov 25, 2023
alice-i-cecile
added
A-ECS
|
Thanks for catching this :) |
alice-i-cecile
removed
the
S-Ready-For-Final-Review
There was already a fix (I don't know if that fixed all the issues), but you should be good to go when you rebase |
This example wasn't reporting any ambiguities! Turns out that child schedules don't inherit the ambiguity detection setting. (I think this must be because that "child" relationship doesn't really exist at schedule build time, and is just an emergent property of what happens during some exclusive system.) Changing the modified schedule to be `Update` reports system ambiguities as expected.
nfagerlund
force-pushed
the
nf/nov23-ambiguity-reporting-example
branch
from
be67603
to
ccfafb2
Compare
|
Righto, rebased |
alice-i-cecile
added
the
S-Ready-For-Final-Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
The
nondeterministic_system_orderexample doesn't actually detect and log its deliberate order ambiguities! It should, tho.Solution
Update the schedule label, and explain in a comment that you can't turn it on for the whole
Mainschedule in one go (alas, that would be nice, but it makes sense that it doesn't work that way).