-
Notifications
You must be signed in to change notification settings - Fork 372
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
No singleton: ChannelContext, ChannelBuilder and channel cache #2455
Conversation
Cancelling the build because this is definitely not ready yet. |
337db2c
to
25befe9
Compare
de75051
to
7faa94b
Compare
75256d3
to
0231acd
Compare
0231acd
to
29aee3c
Compare
Note that a lot of cases where |
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.
Well done 🎉 I could not really follow everywhere, but I have not seen anything shocking.
Let's merge this before it starts conflicting with everything.
@@ -69,7 +71,7 @@ namespace mamba | |||
{ | |||
if (ctx.output_params.json) | |||
{ | |||
std::cout << q.find(query).groupby("name").json().dump(4); | |||
std::cout << q.find(query).groupby("name").json(pool.channel_context()).dump(4); |
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.
std::cout << q.find(query).groupby("name").json(pool.channel_context()).dump(4); | |
std::cout << q.find(query).groupby("name").json(channel_context).dump(4); |
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.
And similar below.
Both
ChannelBuilder
and channel cache are now fusioned intoChannelContext
which is now responsible for storing and providingChannel
instances, and is not a singleton anymore.