-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add default upcasting convert()
method
#444
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.
I really like this idea and the implementation looks reasonable to me.
I mostly commented with general S7 design thoughts 😄
Thanks for doing this. It does seem to resolve #420. |
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 also like the UI with convert()
both for up- and down- casting very much.
I've not studied the changes in detail, but also agree with Hadley's focus on thinking about encapsulating reusable functionality into helper functions .. which are hidden for now but may well be exported to the advanced S7 users eventually (and in part).
Closes #430.
Maybe also closes #420?