Hey everyone, This is my review of the proposed Aedis library. Please take all opinions with a grain of salt.
What is your evaluation of the design?
It's by and large idiomatic Asio down to a T. If one knows Asio well, this library will integrate perfectly with all their existing code and idioms. Though I find myself perplexed by the documentation suggesting the necessity for having reconnections while then delegating that responsibility to me, the user. In the ideal, this would be abstracted from me with an opt-in way of me handling reconnecting manually but as long as users are comfortable copy-pasting from the docs and example code, this is a non-issue. The design emphasizes parsing directly into structures which I enjoy. This is also the only library I've seen that puts PMR front-and-center in its API which I do like. C++17 added PMR for a reason, it's good to see authors taking advantage of this and I think it reflects the innovative spirit of Boost that I've grown to appreciate and admire. The design seems to preclude allowing users to handle I/O on their terms which I dislike. The lowest level interface is `resp3::read()` which is dependent upon Asio concepts. Otherwise, the design is simple and effective.
What is your evaluation of the implementation?
The implementation is actually quite clean and much like its presented interfaces, is idiomatic Asio down to a T. I'm not overwhelmingly familiar with the implementation details as I haven't spent a lot of time with it but after looking at the connection_base class, I feel like I'd be able to debug this library given my level of Asio expertise. Debuggability is paramount when evaluating quality of implementation. The code is laid out quite nicely and uses names effectively. I have no trouble reading it.
What is your evaluation of the documentation?
Stylistically it sets a new standard for how Boost libraries should look and feel. The examples are clear and concise and cover a wide-range of potential use-cases. It's not clear from the documentation if standalone Asio is supported. I think that would be a big user concern.
What is your evaluation of the potential usefulness of the library?
It's useful in the sense that if you need a working Redis client, this fits the bill. While tightly integrating with Asio is nice, this library is almost a tad too late. Having deployed Asio applications and servers, if I had needed a Redis client I would've already been using one of the existing clients as noted on the official Redis website. This library would then require me gradually migrate our existing codebase towards Aedis which requires buy-in from one's team and also from the business. Depending on things like team size and how the company internally manages dependencies and deployments, this is either a non-issue or can create a lot of friction. I've worked on a small team before and migrated us from nlohmann JSON to Boost.JSON, much to the chagrin of my coworkers who didn't view the shift as justified. For greenfield projects looking to use Asio, this library is the most obvious choice.
Did you try to use the library? With what compiler? Did you have any problems?
I did not try to use the library aside from pulling it down and building and running its examples and tests. I used modern compilers (gcc-12) and ran into no problems.
How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I put in a moderate amount of effort. Historically I've never been too motivated to add Redis to applications I've developed. I did, however, study the resp3 protocol to get a better feel for how the library could be designed around it.
Are you knowledgeable about the problem domain?
I'm not very knowledgeable about in-memory key-value stores in general. I'm very familiar with networking and using "advanced" APIs for working with myriad protocols.
Do you think the library should be accepted as a Boost library?
I'm going to vote REJECT. In my view, if there's room in Boost for a Redis library, it would be one that exposes a low-level protocol library similar to Beast or OpenSSL with its linked BIO pairs. This would enable the library to be I/O runtime-agnostic and would still permit the library author to provide an integration with Asio like it has today. Second, the value of this library being included in Boost isn't clear to me. No libraries currently in Boost would benefit from Aedis' inclusion. Aedis benefits somewhat from being in Boost as it can then evolve in lockstep with Asio but unless Asio makes massive changes to its core Stream abstractions, it's safe to say that this library will be stable outside of Boost. While reddit is not necessarily a kind place that reflects the state of C++ developers universally, the reddit thread: https://www.reddit.com/r/cpp/comments/10cnc99/candidate_boostaedis_review_st... did not seem to generate significant user interest in the library being accepted. Interestingly, most of the reddit thread is dedicated to discussing monolithic Boost. While largely non-productive (as most reddit conversations are), we do see that many users are not excited about Yet Another Boost Library. If there's strong user engagement somewhere else, I'll change my REJECT to an ACCEPT but until then, I don't believe Boost itself benefits from Aedis and I don't believe users benefit from Aedis being in Boost outside of those wishing to use Boost as a package manager. Instead, I'd opt into writing a vcpkg portfile and a Conan recipe for this library and distribute it that way. Users will still undoubtedly benefit from this library. Code Review Notes: * fix TODOs in the test suite before requesting formal Boost review * any parser facing the network needs fuzzing * the section comparing Aedis to redis++ should be first and foremost in the docs as it's the first question people looking for a redis client are going to ask - Christian