-
Notifications
You must be signed in to change notification settings - Fork 322
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
try using routefinder #802
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.
The tradeoffs seem reasonable; I'd be in favor of merging this
@jbr Status of this? |
I feel good about this personally, but have hesitations about switching over to it without anyone else playing with it at all, since it's a ground-up rewrite. This is technically a breaking change but I don't have a great sense of how many people would notice any change other than a perf improvement |
If you can fix the rebase I'll see if I can use it a bit (not quite sure when tho) |
@Fishrock123 rebased |
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 feel pretty strongly that we should go forward with this as part of a 0.17
Released as 0.17.0-beta.1 |
routefinder represents a complete rewrite of the route-recognizer behavior
Breaking changes:
There may be an incidental performance improvement of several microseconds per request
closes #783
a step towards #797