aspotify
aspotify copied to clipboard
Wrap reqwest client in Arc
as discussed in #12 I propose wrapping the reqwests client in an Arc
to make in clonable
when merging, can u please label this pr with hacktoberfest-accepted
?
Oh no, that's not what I meant 😅. Reqwest's Client
is already internally ref-counted and cheaply clonable, so it's never needed to Arc
it - I was proposing to internally Arc
the entire aspotify::Client
, so that ClientCredential
s gets Arc
-ed (since that's the thing that is expensive to clone).
when merging, can u please label this pr with
hacktoberfest-accepted
?
Sure! Should I just create a GitHub label then?
So you want to return an Arc
-ed Client
from building it?
Sure! Should I just create a GitHub label then?
That would be perfect! thx
So you want to return an
Arc
-edClient
from building it?
No, the idea is to do what Reqwest does: have Client
internally just contain an Arc<ClientInner>
. ClientInner
is private and contains all the fields that Client
currently does. That way cloning a Client
is just a matter of incrementing the refcount of the ClientInner
.
If new
would return a pre-made Arc<Client>
, we might as well just tell users to make Arc<Client>
themselves - the only benefit of this change would be the convenience of not having to import Arc
.