-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
asio-grpc: add version 3.2.0 #22910
base: master
Are you sure you want to change the base?
asio-grpc: add version 3.2.0 #22910
Conversation
This comment has been minimized.
This comment has been minimized.
a7f64cd
to
503edec
Compare
This comment has been minimized.
This comment has been minimized.
@valgur any chance you could get someone else from the team to review this pull request so that it can finally get merged? :) |
This comment has been minimized.
This comment has been minimized.
@valgur sorry to bother you again, but is there any chance some of your teammates have time to review this PR? |
Hi @Tradias first of all, thank you for your contribution! |
This comment has been minimized.
This comment has been minimized.
@perseoGI Thanks for taking the time to review my pull request. Indeed, there is a new version upstream v3.1.0. I created #24470 as well. Possibly to be done after merging this pr or within in. I do not know what works best for you/conan. I am not sure what you mean by |
This comment has been minimized.
This comment has been minimized.
Didnt catch that assignment, will fix it first thing tomorrow morning, sorry! |
Successful compilation logs, can't paste the whole log when it's building, too long
|
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 have now fixed the handling of the local_allocator option, sorry for the noise - last thing would be to check the dependencies for boost and asio to see if they should be bumped.
Do you have any insight @Tradias if things should work should we bump them to the latest version? Is there any other library usually used alongside this one that might conflict if we're not careful?
Thanks a lot for your patience!
This comment has been minimized.
This comment has been minimized.
@AbrilRBS Bumping is fine. asio-grpc is tested against asio 1.17.0 and Boost.Asio 1.85.0 (asio 1.30.2). I am not aware of other libraries that are used alongside asio-grpc. Most end users use asio to do other networking things like http requests, so they might break from the bump but I assume that is expected? |
THanks for the insight, will bump now and approve, thanksa lot for your patience! :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@AbrilRBS I forgot that older versions of asio-grpc are not compatible with the latest version of asio. What to do now? |
@Tradias can I suggest that you simply conditionally branch the requirements for the older version in the |
This comment has been minimized.
This comment has been minimized.
@AbrilRBS I addressed the issues that came with the version bump of dependencies. Any more concerns? |
Conan v1 pipeline ✔️All green in build 10 (
Conan v2 pipeline ✔️
All green in build 10 (
|
@jcar87 This pr has been open for quite a while now, any chance to finally get it merged? |
Specify library name and version: asio-grpc/3.2.0
Asio-grpc v3.0.0 no longer uses
local_allocator_option
. It could have a newbackend_option
in https://github.com/NVIDIA/stdexec but there does not seem to be a recipe for it here in conan-center.Contains #19070
Note, I am the author of asio-grpc.