On Wed, Nov 13, 2024 at 4:31 AM Richard Hodges via Boost < boost@lists.boost.org> wrote:

The library is authored by Klemens Morgenstern (@klemens-morgenstern in the CppLang slack).

This review is already off to a poor start. A valid complaint from the last review is that discussion took place off-list, on Slack. I suggested that future announcements might remind reviewers to have discussions here instead of elsewhere. I see no such reminder here, and instead I see you are making it easier to contact the library submitter on Slack. At least you didn't also provide the link to the workspace invitation page (which is https://cpp.al/slack) Here are the things being said in the #boost channel regarding this library submission: "404 not found in review email link https://anarthal.github.io/mysql/index.html" "is the reference doc supposed to be complete? E.g. https://klemens.dev/sqlite/structboost_1_1sqlite_1_1connection.html is just mostly empty, except for decorations. For what is worth, I wanted to know how to open the DB read-only, or using URIs, and was thus looking at the doc, and I find none." "these docs are so complete one gets tired of completeness when reading them" "Should these questions be asked on the list instead of here?" "dunno. I'm asking a simple question so far, on expectations I should have" "Expect what you would expect from a complete Boost library." "I was expecting at least the signatures of functions (Ctors in this instance), at least. thus I'm suspecting a ref doc (doxygen based?) generation issue, and was inquiring" "Fwiw, I don't think it's inappropriate to ask about the docs like that here because yeah, that seems kind of just like a simple configuration error I'm guessing" "these are good things to ask on the list and you should state your documentation expectations on the list" "I intend to embed the sources in my code base, instead of compiling Boost. Will Boost.SQLite be compatible with my existing Boost 1.84, for its dependencies (if any)?" "darn, all the classes docs aren't there." "explicit connection(handle_type handle, bool take_ownership = true) that reads inscrutably at the call site. I've taken the habit of using e.g. enum class HasOwnership : bool; inline constexpr HasOwnership cTakeOwnership{ true }; ... so the call site reads well. Bad idea? Is that something you'd consider?" "I think these are great topics for the mailing list" "I confess I was for the ML as well, but in practice, for small things like that, I prefer the interaction here..." "regarding the enum-class-bool idiom, I guess there's .take_ownership = true to make it readable, but C++ doesn't have that for args, does it? Only starting with C++20 for structs, no?" "Well I don't know, introducing a type for a single parameter seems a bit overkill. You usually onl nee the take_ownership = false from plugin code" "I see. In a similar vein, you've decided not to make flags type-safe? E.g. int flags=SQLITE_OPEN_READWRITE|SQLITE_OPEN_CREATE" "right, because the sqlite flags are good enough. i didn't want to wrap everything and duplicate a bunch of code for little gain. the idea is more that the library is an extension of sqlite into C++, not a 100% wrapper" "What about opening a connection with a custom VFS? I've seen that in our code-base. Did you get into that? Both allowing using a custom VFS, and wrappers for VFSs similar to your vtable one?" "nope. I thought about it but couldn't come up with an example that was contrived. I might add it later if someone comes with a use-case" "The designated initializer doesn't work." "Requiring people to move all discussions to the ML is unnecessary friction that benefit a few at the inconvenience of many." "The point is to create an auditable paper trail so people can see why a library was accepted But for something like this where it's: hey, I think you messed up a path, it's not v. pertinent to the quality of the library being reviewed" "I think its very pertinent. The review evaluates the author as much as the library if not more so, since basically Boost is getting married to the author. A consistent pattern of paying great attention to small details such as paths is meaningful." "docs are updated" "The fact is that the previous review was stained by significant discussion taking place off-list. This was discussed after the fact, and now we are repeating this behavior. I agree there is less friction on Slack but I also like having robust reviews of the kind that can be referred to in the future " "Q&A should only be on the list if it's of value to other list members. if a reviewer asks questions for his own education so that he can determine whether to accept the library, these questions do not have to be on the list, although it can be useful for them to be if other reviewers are likely to have the same questions" "connection::prepare has two overloads, and I supposed the 1-arg one throws? The doc doesn't say. (it's kinda obvious, given the 3-args one, but still). Or perhaps there's a global mention in the doc somewhere about the throw-vs-nothrow overloads? (didn't read the manual yet)" "interactive Q&A here is less intimidating than writing to the ML, and more "instant", especially for small questions like mine so far." "nothrow for those overloads is not entirely correct, because the error-code overloads could still throw things like ENOMEM. it basically means that the failure of the main operation is not thrown" Should these discussions stay on Slack or should they be brought to the list? Thanks