-
Notifications
You must be signed in to change notification settings - Fork 182
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
Batteries-included baked provider #3549
Conversation
cc31035
to
d4cf41a
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.
We should probably spend more time organizing the baked_exporter code, but this is also on par for syn code anyway :/
Actual changes look good. Would be kinda cool if we could speed up the repeated binary search by using the failed binary search index as a quick check first. It's a bit complicated to get right, though, so not worth doing here.
(also: so glad that the baked code is formatted again; this was so much easier to review)
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.
My concerns about the batteries-included bake provider are still the same as they were on the other PR.
Given that this PR is merged, let's incorporate this into the summit discussions on the rest of the fallbacking provider infrastructure
Note that we did agree on this:
|
Huh. I wish that discussion included more notes about how we arrived at that conclusion. |
Blocked on #3487Part of #3487