-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Refactor build_schedule and related errors #9579
Refactor build_schedule and related errors #9579
Conversation
- split into separate functions - clean up error flow
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 like chunking this out into functions with clear purposes here,and storing all of the error information in strings on the variants is a much more reasonable architecture than only doing this for some variants.
Note that this is breaking though: ScheduleBuildError
now has strings in more of its variants.
It looks like your PR is a breaking change, but you didn't provide a migration guide. Could you add some context on what users should update when this change get released in a new version of Bevy? |
added a migration guide |
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.
Looks good to me. I'm all for making this code easier to understand.
assert!(matches!( | ||
result, | ||
Err(ScheduleBuildError::DependencyCycle(_)) | ||
)); |
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.
Really wish we had assert_matches!
Objective
Changelog
build_schedule
to be easier to readSample Error Messages
Dependency Cycle
Hierarchy Cycle
System Type Set
Hierarchy Redundancy
Systems have ordering but interset
Cross Dependency
Ambiguity
Migration Guide
ScheduleBuildError
now has strings in more of its variants. You may need to adjust code that is handling these variants.