-
-
Notifications
You must be signed in to change notification settings - Fork 71
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
feat(types): Traceless types #453
Conversation
597b727
to
ccb5c11
Compare
Pull Request Test Coverage Report for Build 8489083056Details
💛 - Coveralls |
eebe170
to
72e9653
Compare
Final bench results
|
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.
Done an initial skim while on the road: need a laptop to get a full review done
198e166
to
d42f3fa
Compare
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.
LGTM 👍
as for memoizing type objects in functions as mentioned above, I'm okay with it if it brings us a performance boost, otherwise i do not see the need for it.
Yeh, can be done later on too, to perhaps do the iteration in a dedicated way. |
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.
LGTM!
@azjezz WDYT about releasing 3.x
with just this breaking change in? It's substantial enough, but allows moving forward easily, IMO :)
Added to my bucket of things to do tomorrow :) |
Allright - looks like this one is ready to ship then.
Lets not rush 3.x. Maybe we can look for other issues / components / things that need these kind of breaking changes first.
I'm planning to play around with the memoizing part soon as well so that I can share some benchmarks there as well. We'll see what gives. |
Fixes #412
Closes #429
This PR will:
This will result in errors that look like:
With an exception stack like:
The benchmarks results look promising: