-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: create Service trait #2920
feat: create Service trait #2920
Conversation
bfb20c3
to
99ed036
Compare
Ok, I see. You could leave the |
I might be doing something wrong, but it's impossible to just replace the @seanmonstar I could use some guidance here. I saw you did some work on removing |
1c71ec6
to
e7c6b5a
Compare
Alright, with the help of cross-cutting removals of code Sean have done to the Looks good to me now, @seanmonstar I'd ask for CI run and if it's all green (except for |
Dumb mistake, I fought the compiler for so long I forgot to run the tests once the project finally compiled. Locally, all the tests (including examples) pass now. There is only one reference to |
b2f8685
to
7c7055c
Compare
I can do |
Alright, couldn't stop myself from removing the last I'm certain pipelines will pass, so I'll wait for the decision and CR. |
e229e10
to
6cc8957
Compare
Juuust one more CI trigger. |
fa8d0c5
to
66384fe
Compare
@seanmonstar Could you address #2920 (comment) and #2920 (comment), and I'll move this PR forward? |
66384fe
to
fae97ce
Compare
…e on tower::Service
…vice on client::conn::SendRequest
…lified for both Client and Server
@seanmonstar I introduced changes from the discussion threads. LGTM, let's wrap and merge this as soon as possible, as the merge conflicts on this branch are killing me 😜 |
…fix error in doc-test
CI failed on a doc-test, this should be fixed now. |
Thanks so so so much! I know this dragged on a while, but it turns out there were tricky things for us to figure out (and conflicts as all sorts of other things got yanked). |
Closes #2853.