-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[Merged by Bors] - Add FromReflect for Visibility #6410
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.
How does FromReflect
work for Parent
/Children
? Doesn't it assume a default value? What does this open up?
I'm also a bit reluctant to add this for those because it allows a way to construct those types (even if it's obviously kinda overwrought), which people could abuse to create a malformed hierarchy.
No comments about the Visibility
impl; I would merge that as trivial.
If I understand correctly, It creates a "default" values using |
Yeah, remove those for now, we can add them later if needed. |
Okay, done! |
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.
bors r+
# Objective - `Visibility` don't have `FromReflect` derive. ## Solution - Add it. --- ## Changelog ### Added - `FromReflect` for `Visibility`.
# Objective - `Visibility` don't have `FromReflect` derive. ## Solution - Add it. --- ## Changelog ### Added - `FromReflect` for `Visibility`.
Objective
Visibility
don't haveFromReflect
derive.Solution
Changelog
Added
FromReflect
forVisibility
.