-
Notifications
You must be signed in to change notification settings - Fork 572
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 support for Class and Object on top of Properties. #3489
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.
This is a pretty large discussion topic but I am not a huge fan of the current untyped approach using dynamic. The entire purpose of Definition/Instance was to give this sort of API while maintaining type safety. I really think we should just bite the bullet and make D/I be the intended way of defining and instantiating Classes.
Now I understand that we have some use cases that require an unsafe API for just instantiating classes by name when you certainly are not going to know anything about the actual typed interface. But rather than making the normal API embrace this untyped route, I think we can just implement that particular API directly without having its untyped-ness impact the more typical user API
val inst: DynamicObject = Class.unsafeInstanceFromName("MyClassName"))
Depending on how hard it is to implement D/I for classes, we could punt on implementing it fully and just make Definition[Class]
work, then use this unsafe API as the only API to instantiate an object for the short term.
858f7ff
to
95c97d8
Compare
c27c15a
to
ba3478e
Compare
Classes can only be elaborated with Definition. Instantiation via D/I is not yet supported--current instantiation is via an unsafe "by name" API.
ba3478e
to
7eba073
Compare
Contributor Checklist
docs/src
?Sending a draft now, thought it isn't quite ready.
Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.5.x
,3.6.x
, or5.x
depending on impact, API modification or big change:6.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.