async-mqtt5 is CONDITIONALLY ACCEPTED
Hi all, thanks to everyone participating in the review! I really want to express my appreciation to the reviewers, especially those who invested multiple hours if not days to review this library: thank you Vinnie, Ruben, Mohammad and Marcelo. I interpreted Darylls review as a "weak rejection", but since that doesn't exist in the formal review process I'll count it as a rejection. Leaving the recommendations at 1 ACCEPT 3 CONDITIONAL ACCEPT 2 REJECT The actual conditions for acceptance are: 1. Correct usage of the executor. It is an asio contract that the async operation always completes on the associated executor and mqtt5 should follow this. 2. Add logging/reporting facilities. A user should be able to follow what is happening internally and ideally react to it, especially concerning reconnects. I also recommend the authors to review all the comments, including the rejections (Daryll's review had interesting details) and non-review comments such as from Peter Turcan. I would also advise that inclusion in boost is usually a time to change the name, because the library now gains the boost. prefix. I.e. boost.async-mqtt5 might be a bit cumbersome. Also note that there is no time requirement to include this library in boost in the next release. Thanks again to everyone participating in the review and to Ivica & Mireo for their submission.
I worry about the integrity of the Boost review process. The review manager and all the accepting reviewers were/are affiliated with the C++ Alliance. The review manager thanks these reviewers, but not the ones who rejected. The outcome of the review is presented as a foregone conclusion, with no analysis of the actual topics that have been discussed. The review manager does not state how the different reviews and issues have weighed in on his decision. There is at least one accepting review where the reviewer hasn't even built and ran a single example. Meanwhile, the two rejections (and some non-review comments) brought up major design questions which haven't been addressed by either the review manager or the author(s). These facts make the outcome way less clear, in my opinion.
On Wed, Oct 30, 2024 at 4:04 PM Thomas Fowlery
I worry about the integrity of the Boost review process. The review manager and all the accepting reviewers were/are affiliated with the C++ Alliance.
That is incorrect, Marcelo is not and has never been associated with the C++ Alliance. I would recommend getting facts like this correct when discussing integrity.
The review manager thanks these reviewers, but not the ones who rejected.
I thanked all the reviewers, but pointed out those who invested more than 8h. The rejecting reviewers did not state how much time they invested. I did state how much I appreciated Daryll's review and all the detail he went into.
The outcome of the review is presented as a foregone conclusion, with no analysis of the actual topics that have been discussed.
The outcome of the review is the conclusion of the review.
The review manager does not state how the different reviews and issues have weighed in on his decision. There is at least one accepting review where the reviewer hasn't even built and ran a single example. Meanwhile, the two rejections (and some non-review comments) brought up major design questions which haven't been addressed by either the review manager or the author(s). These facts make the outcome way less clear, in my opinion.
That's a fair point, so let me add to this: The two rejections were essentially based on the fact that async-mqtt5 is an asio-only library and too high level. In either case the reviewers would want a different kind of library than under review. Four reviewers thought the basic idea of the library (a high level client based on asio) was the right idea, so that's a 4:2 - I wouldn't know how to weigh a basic preference like this. Otherwise I put the most weight on Marcelo's & Ruben's reviews on the Accept side, as they are authors of asio-based libraries, have invested a fair amount of time & pointed out some significant details (showing they really looked into it). On the Reject side I weighted Daryll's review most highly, as he's experience with both asio & mqtt and his review gave the impression that he really looked deeply into the library. I hope that helps to alleviate your worries.
Hi everyone, On behalf of the Mireo team, I’d like to express our gratitude to the Boost community for the inclusion of Async.MQTT5 (or even better boost.mqtt5) into Boost. Special thanks to Klemens for managing the review process, and to all the reviewers for their valuable comments, questions, and detailed feedback! Async.MQTT5 was designed to be extremely easy to use while still allowing for some customization. Our goal was to abstract away most MQTT protocol details and network management to the greatest extent possible, making it accessible to developers with minimal MQTT knowledge. Naturally, this approach may make the library less suitable for advanced use cases where complete control over the protocol is required. However, we hope it will serve a wide range of use cases where projects can benefit from a plug-and-play component that “just works.” Having the library included in Boost certainly strengthens the confidence of developers looking to rely on it. Thank you once again, Ivica Siladic
On 30.10.2024., at 09:45, Klemens Morgenstern via Boost
wrote: On Wed, Oct 30, 2024 at 4:04 PM Thomas Fowlery
wrote: I worry about the integrity of the Boost review process. The review manager and all the accepting reviewers were/are affiliated with the C++ Alliance.
That is incorrect, Marcelo is not and has never been associated with the C++ Alliance. I would recommend getting facts like this correct when discussing integrity.
The review manager thanks these reviewers, but not the ones who rejected.
I thanked all the reviewers, but pointed out those who invested more than 8h. The rejecting reviewers did not state how much time they invested. I did state how much I appreciated Daryll's review and all the detail he went into.
The outcome of the review is presented as a foregone conclusion, with no analysis of the actual topics that have been discussed.
The outcome of the review is the conclusion of the review.
The review manager does not state how the different reviews and issues have weighed in on his decision. There is at least one accepting review where the reviewer hasn't even built and ran a single example. Meanwhile, the two rejections (and some non-review comments) brought up major design questions which haven't been addressed by either the review manager or the author(s). These facts make the outcome way less clear, in my opinion.
That's a fair point, so let me add to this: The two rejections were essentially based on the fact that async-mqtt5 is an asio-only library and too high level. In either case the reviewers would want a different kind of library than under review. Four reviewers thought the basic idea of the library (a high level client based on asio) was the right idea, so that's a 4:2 - I wouldn't know how to weigh a basic preference like this.
Otherwise I put the most weight on Marcelo's & Ruben's reviews on the Accept side, as they are authors of asio-based libraries, have invested a fair amount of time & pointed out some significant details (showing they really looked into it). On the Reject side I weighted Daryll's review most highly, as he's experience with both asio & mqtt and his review gave the impression that he really looked deeply into the library.
I hope that helps to alleviate your worries.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Wed, Oct 30, 2024 at 1:04 AM Thomas Fowlery via Boost < boost@lists.boost.org> wrote:
The outcome of the review is presented as a foregone conclusion, with no analysis of the actual topics that have been discussed. The review manager does not state how the different reviews and issues have weighed in on his decision.
Unfortunately I agree. I expect there to be a well written summary of all the points that were presented by the reviewers, including the negative ones.
Meanwhile, the two rejections (and some non-review comments) brought up major design questions which haven't been addressed by either the review manager or the author(s).
One of the rejections was low-effort and misses the mark in terms of why libraries are accepted. I assume it was rightly discounted accordingly, yet this was the opportunity for the review manager to better explain why so that future reviews may avoid those mistakes. Thanks
This is an unfortunate miscarriage of a Boost review. The author of the proposed library avoided all discussions about the design and then the library was prematurely accepted while the review was, honestly, still active. I think we should petition the review wizards to contest this result and that we find a new review manager. - Christian
On Wed, Oct 30, 2024 at 7:01 AM Christian Mazakas via Boost < boost@lists.boost.org> wrote:
The author of the proposed library avoided all discussions about the design
There was quite a lot of discussion regarding the design, and it took place on Slack rather than the mailing list. Thanks
On 10/30/24 17:14, Vinnie Falco via Boost wrote:
On Wed, Oct 30, 2024 at 7:01 AM Christian Mazakas via Boost < boost@lists.boost.org> wrote:
The author of the proposed library avoided all discussions about the design
There was quite a lot of discussion regarding the design, and it took place on Slack rather than the mailing list.
I think, the formal media for the review discussion is the mailing list. Yes, reviews may be collected through other channels, but the discussion needs to be held in one place that is easily referenceable, and this is currently the ML. I'm not invalidating any points that may have been made on Slack or elsewhere, but I'm saying that the fact that the discussion was held outside the ML is an organizational issue that should probably have been prevented by the review manager. I'm not subscribed to Slack and I imagine, there are other users who also aren't subscribed. AFAIK, Slack spaces are not viewable by non-subscribers, which make it unsuitable for referenceable discussions such as Boost reviews.
Hi everyone, Thank you to the community, reviewers, and the review manager for all the hard work and dedication. I apologize for not being available to review the library myself, as I would have liked to contribute. I don’t have strong opinions on the review process. Most of the feedback seems to center around communication styles and individual preferences. I don't agree with the suggestion of replacing the review manager; that would overlook the significant work done by the authors, reviewers, and manager. If the process wasn’t perfect, perhaps it just needs clearer formalization and communication. To my knowledge, there isn't a step-by-step document outlining the specific responsibilities and tasks for a Boost review manager. If such "Boost Review Manager Guidelines" don’t exist or is not clearly formalized, I find it unfair to critique the contributions of those who have dedicated their time. Is there an existing document that clearly articulates and communicates these responsibilities to the review manager as checkboxes for example? I’m also unaware of any guideline stating that all communications related to a library under review should only occur on the mailing list. I don't think such a rule would even be practical. The mailing list and Slack serve different purposes for communication. Personally, I would consider unsubscribing from the mailing list if Slack-style synchronous messages started spamming my inbox. Each communication channel serves a unique purpose, and it’s up to community members to subscribe to those that best suit their needs. If someone chooses to intentionally avoid a particular channel, that's their choice, but it’s unfair to fault others for using it. While I agree that the review manager could have included more details about Slack discussions in their report, this again raises the question: Are these guidelines relevant, clearly defined, and communicated? Best regards, and rainbow kitties Arno On Wed, Oct 30, 2024 at 3:32 PM Andrey Semashev via Boost < boost@lists.boost.org> wrote:
On Wed, Oct 30, 2024 at 7:01 AM Christian Mazakas via Boost < boost@lists.boost.org> wrote:
The author of the proposed library avoided all discussions about the design
There was quite a lot of discussion regarding the design, and it took
On 10/30/24 17:14, Vinnie Falco via Boost wrote: place
on Slack rather than the mailing list.
I think, the formal media for the review discussion is the mailing list. Yes, reviews may be collected through other channels, but the discussion needs to be held in one place that is easily referenceable, and this is currently the ML.
I'm not invalidating any points that may have been made on Slack or elsewhere, but I'm saying that the fact that the discussion was held outside the ML is an organizational issue that should probably have been prevented by the review manager.
I'm not subscribed to Slack and I imagine, there are other users who also aren't subscribed. AFAIK, Slack spaces are not viewable by non-subscribers, which make it unsuitable for referenceable discussions such as Boost reviews.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Wed, 30 Oct 2024 at 16:00, Arnaud Becheler via Boost < boost@lists.boost.org> wrote:
I’m also unaware of any guideline stating that all communications related to a library under review should only occur on the mailing list.
FYA, https://www.boost.org/community/reviews.html#Introduction says "a formal review, where Boost mailing list members comment on their evaluation of the library" "Boost mailing list members are encouraged to submit Formal Review comments: - Publicly on the mailing list. - Privately to the Review Manager." No other communication channels are mentioned, listed or considered for review. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
On 10/30/24 18:00, Arnaud Becheler wrote:
On Wed, Oct 30, 2024 at 3:32 PM Andrey Semashev via Boost
mailto:boost@lists.boost.org> wrote: On 10/30/24 17:14, Vinnie Falco via Boost wrote: > On Wed, Oct 30, 2024 at 7:01 AM Christian Mazakas via Boost < > boost@lists.boost.org mailto:boost@lists.boost.org> wrote: > >> The author of the proposed library avoided all discussions about the design > > There was quite a lot of discussion regarding the design, and it took place > on Slack rather than the mailing list.
I think, the formal media for the review discussion is the mailing list. Yes, reviews may be collected through other channels, but the discussion needs to be held in one place that is easily referenceable, and this is currently the ML.
I'm not invalidating any points that may have been made on Slack or elsewhere, but I'm saying that the fact that the discussion was held outside the ML is an organizational issue that should probably have been prevented by the review manager.
I'm not subscribed to Slack and I imagine, there are other users who also aren't subscribed. AFAIK, Slack spaces are not viewable by non-subscribers, which make it unsuitable for referenceable discussions such as Boost reviews.
I don’t have strong opinions on the review process. Most of the feedback seems to center around communication styles and individual preferences. I don't agree with the suggestion of replacing the review manager; that would overlook the significant work done by the authors, reviewers, and manager. If the process wasn’t perfect, perhaps it just needs clearer formalization and communication.
For the record, I did not suggest to replace the review manager or disregard the review conclusion. My comment was entirely about communication channels used during the review and nothing else. It may be that this communication issue could be mitigated if the review manager posts the discussion records on the ML, at least those he considers important for the purpose of the review. But of course it's up to the review manager whether he thinks that is something worth doing.
To my knowledge, there isn't a step-by-step document outlining the specific responsibilities and tasks for a Boost review manager.
https://www.boost.org/community/reviews.html See the "Notes for Review Managers" section in that document.
I’m also unaware of any guideline stating that all communications related to a library under review should only occur on the mailing list.
From the page linked above:
<quote> Boost mailing list members are encouraged to submit Formal Review comments: * Publicly on the mailing list. * Privately to the Review Manager. Private comments to a library submitter may be helpful to her or him, but won't help the Review Manager reach a decision, so the other forms are preferred. </quote>
I don't think such a rule would even be practical. The mailing list and Slack serve different purposes for communication. Personally, I would consider unsubscribing from the mailing list if Slack-style synchronous messages started spamming my inbox.
Mailing lists are not chats, so chat-style communication would not be suitable for an ML. I think, everyone here understand this. And I consider that a plus point for keeping the review discussions on the ML as this favors more thought out and in-depth discussions.
Each communication channel serves a unique purpose, and it’s up to community members to subscribe to those that best suit their needs. If someone chooses to intentionally avoid a particular channel, that's their choice, but it’s unfair to fault others for using it.
One important correction: given that Slack blocks access based on user's location, not being subscribed might not be the user's choice. And again, being able to reference individual reviews and discussion points is an important part of a review. Slack doesn't allow this, which means it should not be used for conducting review discussions. PS: Please, don't top-post.
Andrey Semashev wrote:
On 10/30/24 17:14, Vinnie Falco via Boost wrote:
On Wed, Oct 30, 2024 at 7:01 AM Christian Mazakas via Boost < boost@lists.boost.org> wrote:
The author of the proposed library avoided all discussions about the design
There was quite a lot of discussion regarding the design, and it took place on Slack rather than the mailing list.
I think, the formal media for the review discussion is the mailing list. Yes, reviews may be collected through other channels, but the discussion needs to be held in one place that is easily referenceable, and this is currently the ML.
I agree with Andrey. Design discussions about a library currently under review are supposed to take place on the mailing list. This serves two purposes, it helps reviewers who for any reason are struggling to reach a final accept/reject verdict, and it can encourage people to review the library by piquing their interest. In addition, having the discussion archived is both of historical interest and helps the review manager write up a summary. Should he deign to do so, of course. Going back to the original question of whether the library author is expected to answer questions about the library under review: in short, yes. This helps reviewers evaluate the soundness of the design decisions and get a feel about how the library has evolved. E.g. a potential reviewer might ask "I see that you're doing X, but have you considered doing W instead?" and the author would reply "yes, we used to do W, but this turned out to not be a good idea in practice because of reasons A and B, and because users tended to fall into trap C, which X avoids."
On Wed, Oct 30, 2024 at 10:11 AM Peter Dimov via Boost < boost@lists.boost.org> wrote:
I agree with Andrey. Design discussions about a library currently under review are supposed to take place on the mailing list. This serves two purposes, it helps reviewers who for any reason are struggling to reach a final accept/reject verdict, and it can encourage people to review the library by piquing their interest.
In hindsight I wish the discussions had taken place on the mailing list. Yet there was far less friction on Slack and ultimately I chose the path of least resistance, as the immediacy of replies in the moment led to a more engaging conversation. Perhaps we might explore how these two separate platforms can be combined to create something greater than each individual part? Peter how do we fix this? Thanks
On Tue, Nov 5, 2024 at 2:29 PM Vinnie Falco via Boost
On Wed, Oct 30, 2024 at 10:11 AM Peter Dimov via Boost < boost@lists.boost.org> wrote:
I agree with Andrey. Design discussions about a library currently under review are supposed to take place on the mailing list. This serves two purposes, it helps reviewers who for any reason are struggling to reach a final accept/reject verdict, and it can encourage people to review the library by piquing their interest.
In hindsight I wish the discussions had taken place on the mailing list. Yet there was far less friction on Slack and ultimately I chose the path of least resistance, as the immediacy of replies in the moment led to a more engaging conversation. Perhaps we might explore how these two separate platforms can be combined to create something greater than each individual part? Peter how do we fix this?
I think this was the first review with discussions like this on slack. The reason most likely was that we already had an active boost-mqtt group on there. Additionally a lot of pre-review discussion happened there. The impression that Ivica was not responsive is partly caused by this. I didn't take the slack discussion into account, nor participate because we didn't have a preset policy for this. Here's what I would make policy, if the author wants to be active on slack: 1. A review announcement includes a slack channel designated for discussion related to the review 2. Require reviews to be on the mailing list (or by mail to the RM). 3. At the closing of the review period the RM sends the transcript to the ML. 4. The RM is allowed to post parts of the transcript earlier (e.g. a daily update) depending on the activity on slack. By closing the whole discussion must be on the ML. This would make Slack an option but keep a complete & public record on the ML. Maybe the new website has a way to make this a bit easier than copy-pasting from slack.
Thanks
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Mon, Nov 4, 2024 at 10:59 PM Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
3. At the closing of the review period the RM sends the transcript to the ML.
Can you please construct the Slack transcript for async-mqtt5 and post it so we can see what that looks like? Thanks
On Tuesday, November 5, 2024, Vinnie Falco wrote:
On Mon, Nov 4, 2024 at 10:59 PM Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
3. At the closing of the review period the RM sends the transcript to the ML.
Can you please construct the Slack transcript for async-mqtt5 and post it so we can see what that looks like?
And then what happens next? e.g. Are we going to restart the review if the transcript posted to the ML results in significant new opinions and information by mailing list members that would change the review manager's opinion? I really hope I have misunderstood what transpired here, because it sounds like the Boost community on the mailing list did not have time during the designated review period to counter any points made off-list that influenced the review manager. The time to share that information is during the 10 days. Glen
On Tue, Nov 5, 2024 at 2:22 AM Glen Fernandes
On Tuesday, November 5, 2024, Vinnie Falco wrote:
Can you please construct the Slack transcript for async-mqtt5 and post it so we can see what that looks like?
And then what happens next?
No idea. I'm curious to see what such a transcript might look like and what the obstacles are to extracting it. Thanks
On Tue, Nov 5, 2024 at 9:14 PM Vinnie Falco via Boost
On Tue, Nov 5, 2024 at 2:22 AM Glen Fernandes
wrote: On Tuesday, November 5, 2024, Vinnie Falco wrote:
Can you please construct the Slack transcript for async-mqtt5 and post it so we can see what that looks like?
And then what happens next?
No idea. I'm curious to see what such a transcript might look like and what the obstacles are to extracting it.
On slack only a workspace owner or admin can export this, which would be much better than copy-paste. I am neither, can you get someone to do the export for the review period?
Thanks
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 11/5/24 09:59, Klemens Morgenstern via Boost wrote:
Here's what I would make policy, if the author wants to be active on slack:
1. A review announcement includes a slack channel designated for discussion related to the review 2. Require reviews to be on the mailing list (or by mail to the RM). 3. At the closing of the review period the RM sends the transcript to the ML. 4. The RM is allowed to post parts of the transcript earlier (e.g. a daily update) depending on the activity on slack. By closing the whole discussion must be on the ML.
This would make Slack an option but keep a complete & public record on the ML.
Maybe the new website has a way to make this a bit easier than copy-pasting from slack.
The problem with reproducing the Slack discussion on the ML is that it doesn't allow further discussion between residents of ML and Slack. It is better than nothing, of course, but I don't see it as something that we want to encourage. I think, a better policy would be for the review manager to state clearly before the review that the review discussion happens on the ML *only*, and any discussions on other platforms, including Slack, will not be taken into account. The review manager is allowed to post comments from other platforms on the ML if he is willing to make the audience aware of the points made there, and possibly receive feedback on those points from the library author and community. This would be the means for the review manager to gather additional input to make up his decision. This would *not* be the means to participate in the review outside the ML, though. There was a precedent to this during the review of Boost.Scope. Dmitry has posted comments gathered from Reddit, which I answered, and some of which eventually took to improve the library. I'm not sure if it helped Dmitry to manage the review, but I would hope it provided some answers and benefited the library.
On Tuesday, November 5, 2024, Vinnie Falco wrote:
On Wed, Oct 30, 2024 at 10:11 AM Peter Dimov via Boost < boost@lists.boost.org> wrote:
I agree with Andrey. Design discussions about a library currently under review are supposed to take place on the mailing list. This serves two purposes, it helps reviewers who for any reason are struggling to reach a final accept/reject verdict, and it can encourage people to review the library by piquing their interest.
In hindsight I wish the discussions had taken place on the mailing list. Yet there was far less friction on Slack and ultimately I chose the path of least resistance, as the immediacy of replies in the moment led to a more engaging conversation.
I'm not sure I understand. Is what happened here that the review result was driven (even if in part) by information provided to the review manager _outside_ of the mailing list, and this information was _not_ shared by the review manager with the mailing list? If so, I would be surprised if more people aren't upset with the review. Glen
On Tue, 5 Nov 2024 at 11:14, Glen Fernandes via boost@lists.boost.org wrote:
On Wed, Oct 30, 2024 at 10:11 AM Peter Dimov via boost@lists.boost.org wrote:
I agree with Andrey. Design discussions about a library currently under review are supposed to take place on the mailing list. This serves two purposes, it helps reviewers who for any reason are struggling to reach a final accept/reject verdict, and it can encourage people to review
On Tuesday, November 5, 2024, Vinnie Falco wrote: the
library by piquing their interest.
In hindsight I wish the discussions had taken place on the mailing list. Yet there was far less friction on Slack and ultimately I chose the path of least resistance, as the immediacy of replies in the moment led to a more engaging conversation.
I'm not sure I understand. Is what happened here that the review result was driven (even if in part) by information provided to the review manager _outside_ of the mailing list, and this information was _not_ shared by the review manager with the mailing list?
My understanding is this: - review concluded based on what was posted to the mailing list - extensive discussions happened on the Slack, but without any trace in public domain - discussion on the Slack seem to be formally ignored what makes some people surprised However, now, I'm confused myself too. Best regards, -- Mateusz Loskot on behalf of the Review Wizards
On 11/5/24 13:14, Glen Fernandes via Boost wrote:
On Tuesday, November 5, 2024, Vinnie Falco wrote:
On Wed, Oct 30, 2024 at 10:11 AM Peter Dimov via Boost < boost@lists.boost.org> wrote:
I agree with Andrey. Design discussions about a library currently under review are supposed to take place on the mailing list. This serves two purposes, it helps reviewers who for any reason are struggling to reach a final accept/reject verdict, and it can encourage people to review the library by piquing their interest.
In hindsight I wish the discussions had taken place on the mailing list. Yet there was far less friction on Slack and ultimately I chose the path of least resistance, as the immediacy of replies in the moment led to a more engaging conversation.
I'm not sure I understand. Is what happened here that the review result was driven (even if in part) by information provided to the review manager _outside_ of the mailing list, and this information was _not_ shared by the review manager with the mailing list?
If so, I would be surprised if more people aren't upset with the review.
Klemens already stated that he didn't take the discussion on Slack into account in the review conclusion. However, as I've already said, I do think that the discussion should have happened on the ML.
On Tue, 5 Nov 2024 at 11:31, Andrey Semashev via Boost < boost@lists.boost.org> wrote:
However, as I've already said, I do think that the discussion should have happened on the ML.
Legitimate or not, personally, that is also my expectation. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
On Tue, Nov 5, 2024 at 8:31 PM Mateusz Loskot via Boost
On Tue, 5 Nov 2024 at 11:31, Andrey Semashev via Boost < boost@lists.boost.org> wrote:
However, as I've already said, I do think that the discussion should have happened on the ML.
Legitimate or not, personally, that is also my expectation.
I think we need to distinguish three things here: 1. What a review manager takes into account. 2. What reviewer bases his review on. 3. How an author acts. This discussion started on point 3, as Ivica was being accused of dodging questions. In response it was brought up that he was very responsive on Slack. Generally, I think the author's behaviour can be taken into account for a review result, but can't invalidate the review process itself. The solution is writing a review, expressing concerns with the author's lack of responsiveness. The policy for (1) is that an RM may take anything into account, but is only required to take into account reviews that are posted to the ML or sent to his EMail. This is stated as follows (https://www.boost.org/community/reviews.html)
Proposed libraries are accepted into Boost only after undergoing a formal review, where Boost mailing list members comment on their evaluation of the library.
The final "accept" or "reject" decision is made by the Review Manager, based on the review comments received from boost mailing list members.
Boost mailing list members are encouraged to submit Formal Review comments:
- Publicly on the mailing list. - Privately to the Review Manager.
"review comments" refers to what we call review these days, thus the policy states that the RM bases his decision on these. This does not include all comments on the ML. The process explicitly states that this is for members of the ML and I think that's correct and should not include comments on other media. Considering other posts on the ML at his own discretion seems appropriate. Regarding (2): I think it is just a fact of life that reviewers will have discussions with the authors on other platforms. Even if we make it a rule that all discussion about the library has to take place on the ML during the review period, a reviewer may have made up his mind based on slack discussions that happened earlier. I don't think there's a good solution here other than to ask reviewers to include what communication he had with the library author and encourage discussion on the ML. We should define a policy soon, as the next review starts on the 13th with a RM (Richard Hodges) and an author (yours truly) that are both active on slack.
On Tue, 5 Nov 2024 at 15:35, Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
On Tue, Nov 5, 2024 at 8:31 PM Mateusz Loskot via Boost
wrote: On Tue, 5 Nov 2024 at 11:31, Andrey Semashev via Boost < boost@lists.boost.org> wrote:
However, as I've already said, I do think that the discussion should have happened on the ML.
Legitimate or not, personally, that is also my expectation.
I think we need to distinguish three things here:
1. What a review manager takes into account. 2. What reviewer bases his review on. 3. How an author acts. [...]
Thank you for the clarification. [...] We should define a policy soon, as the next review starts on the 13th
with a RM (Richard Hodges) and an author (yours truly) that are both active on slack.
Although I largely agree with your opinion, I don't think it is the right time to touch any of the reviews policies. It certainly is not enough time. I think, there needs to be period of several months allocated for such discussions during which no reviews are conducted, until the Community agrees on any updates. However, some of the instructions and statements in the current documentation on the reviewing process could be improved and further clarified indeed. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
On Thu, 31 Oct 2024, 12:14 am Vinnie Falco via Boost,
On Wed, Oct 30, 2024 at 7:01 AM Christian Mazakas via Boost < boost@lists.boost.org> wrote:
The author of the proposed library avoided all discussions about the design
There was quite a lot of discussion regarding the design, and it took place on Slack rather than the mailing list.
I should probably have read it then? Was this stated to be part of the review process? I did belatedly find it after review was closed, it looks like I wouldn't have felt comfortable participating in it earlier anyway, even had the timing of it aligned. My impression was (and is) slack is "internal" boost discussion. Thanks
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Wed, 30 Oct 2024 at 15:01, Christian Mazakas via Boost < boost@lists.boost.org> wrote:
This is an unfortunate miscarriage of a Boost review.
TL;TR: Sorry, but the critique you expressed is unclear to me.
The author of the proposed library avoided all discussions about the design
AFAIK, there is no requirement that authors of the library under review respond or participate in discussions during ongoing review of their library. Authors could simply respond "Thank you for your review." and this would be a formally acceptable reaction. "Many reviews include questions for library authors. Authors are interested in defending their library against your criticisms; otherwise, they would not have brought their library up for review." is just an assumption of such interest and will to offer reaction. Second, there is nothing that stops reviewers to change their mind before the end of the review period. Reviewers who are unhappy about lack of answers from authors are free to change their mind assuming such silent authors will not be interested or capable to fulfil their maintainer's responsibilities [1]
and then the library was prematurely accepted while the review was, honestly, still active.
You will need to be more specific here.
I think we should petition the review wizards to contest this result and that we find a new review manager.
[1] https://www.boost.org/community/reviews.html Best regards, -- Mateusz Loskot, http://mateusz.loskot.net
On Wed, Oct 30, 2024 at 1:04 AM Thomas Fowlery via Boost < boost@lists.boost.org> wrote:
I worry about the integrity of the Boost review process. The review manager and all the accepting reviewers were/are affiliated with the C++ Alliance.
Perhaps you can help me understand how Alliance-affiliated participants may negatively impact the integrity of the review process? The C++ Alliance derives no benefit from coordinating the results of reviews, or from achieving particular review outcomes. Of course, I prefer that when we invest in producing a new Boost library (for example, Boost.Charconv) that the library is ultimately accepted. And the way to do that is to ensure that the library is great, not to influence the review. If an Alliance Staff Engineer produced a bad library I would not want to see that accepted. Ultimately, the success of C++ Alliance is tied to the success of Boost, and having healthy reviews with sufficient participation and the highest levels of integrity are measures of that success. That said, the async-mqtt5 library was not and is not an Alliance project. Thanks
On Sat, Nov 9, 2024 at 7:52 PM Vinnie Falco via Boost
On Wed, Oct 30, 2024 at 1:04 AM Thomas Fowlery via Boost < boost@lists.boost.org> wrote:
I worry about the integrity of the Boost review process. The review manager and all the accepting reviewers were/are affiliated with the C++ Alliance.
Perhaps you can help me understand how Alliance-affiliated participants may negatively impact the integrity of the review process?
Hi all, About the conflict of interest, maybe the initial remark stems from the fact that there are different expectations about what is a conflict of interest and what role (Review Manager or Reviewer) they affect. Different expectations from a text come from different interpretations of the text, which boils down to the text not being explicit enough. From my perspective, it means: *let's fix the guidelines*. We can do so by giving them a clear *Conflict of Interest* section, for both the *Review Manager *and the *Reviewer* roles. I am personally used to see this kind of clause of non-conflict of interest: 1. I will disclose any personal, professional, or financial relationships with authors, contributors, or stakeholders that could influence my judgment or create a perception of bias, 2. I will not participate in the review process for submissions in which I have contributed as an author or co-author. 3. I will not participate in the review process for submissions in which there exists any form of financial interest or contractual obligation that may compromise my objectivity. 4. I will not participate in the review process for submissions in which the authors are from the same institution, organization, or research group I am currently affiliated with. In this state, it can not be applied to Boost. From what I have seen on the ML there seems to be a confusion between clauses used by the outside world, the clauses formalized by Boost, the way people have done things, potential conflict of interests and perceived conflicts of interests. Let's try to disentangle this. Point 1 is explicitly mentioned in the Review Manager section: In order to avoid any conflicts of interest a potential review manager is expected to disclose to the Boost Community if they have any relationship to the author of the library or the library itself. -> IMO it is reasonable to require it for both Review Manager *and* Reviewers. Point 2 figures in the guidelines (in a diluted form it seems). -> IMO it is reasonable to explicitly require it for both Review Manager *and* Reviewers Point 3 does not figure in the guidelines. -> IMO it could maybe carefully/reasonably be included after careful phrasing, for example to discard the case where an employee of an organization is the Review Manager of their boss's proposal, or N employees weigh in the Review process of their boss's proposal, Point 4 does not figure in the guidelines. -> IMO it could be reasonable for a Review Manager, but can not reasonably figure in the guidelines for a Reviewer, as it would unfairly disadvantage organizations that are deeply involved with Boost. I mean, we all think about the C++ Alliance here, but if Boost loses the reviewing expertise of all its members because one of them happens to write a library that may hinder Boost end quality in the end. That being said, it must be weighted by the total number of external Reviewers. With all of these confusions made explicit, I believe they could be solved by adding something around the following in the Guidelines, and, to ease their onboarding, as checkboxes during the Review Manager and Reviewer submission process. *Conflict of Interest Guidelines:* *Clause for both roles: **All participants in the review process, including Review Managers and Reviewers, must disclose any personal, professional, or financial relationships with authors, contributors, or stakeholders that could influence their judgment or create a perception of bias.* *Clause for Review Manager:* *A Review Manager must not manage the review process for submissions in which (i) **They have contributed as an author or co-author (ii) **They have a financial interest or contractual obligation tied to the submission, (iii) **They are in a hierarchical relationship with one of the authors (e.g., an employee reviewing their manager’s submission).* *Clause for Reviewer:* *A Reviewer must not participate in reviewing submissions in which (i) **They have contributed as an author or co-author, (ii) **They have a direct financial interest or contractual obligation tied to the submission. * *Reviewers affiliated with the same organization as the author may participate, provided they disclose this relationship and their input is balanced by independent reviewers and reported at the discretion of the Review Manager.*This could ensure that organizations deeply involved with Boost, such as the C++ Alliance, can still contribute their expertise while preventing reviews dominated by internal bias. I hope this was useful for future rework of the guidelines, Best wishes and rainbow kitties, Arno
On Sat, Nov 9, 2024 at 12:44 PM Arnaud Becheler
1. I will disclose any personal, professional, or financial relationships with authors, contributors, or stakeholders that could influence my judgment or create a perception of bias,
I must apologize that in the async-mqtt5 review, disclosures were not
1. I will not participate in the review process for submissions in which I have contributed as an author or co-author. 2. I will not participate in the review process for submissions in which there exists any form of financial interest or contractual obligation that may compromise my objectivity. 3. I will not participate in the review process for submissions in which the authors are from the same institution, organization, or research group I am currently affiliated with.
I understand what you are trying to do and there is a better way to do it. Anyone can participate in the reviews, and even individuals who are conflicted should still participate. In most cases they have considerable understanding of the domain and the library in particular, we want to hear what they have to say. However, the review manager should consider all of
made. A couple of staff engineers forgot, and even I forgot. We might consider these changes: 1. The review announcement reminds folks to disclose their affiliation 2. The review manager is suggested to be aware of Alliance staff (listed at https://cppalliance.org/#team) and remind review submissions which are missing disclosures (to be clear, reviewers are still required to remember to disclose their affiliations) 3. The review result can enumerate any affiliations of reviewers as a reminder the above (1, 2, and 3) when evaluating the extent to which a particular reviewer should influence the outcome. For this, conspicuous disclosures are needed (see previous)
Point 1 is explicitly mentioned in the Review Manager section: In order to avoid any conflicts of interest a potential review manager is expected to disclose to the Boost Community if they have any relationship to the author of the library or the library itself.
Yes all the disclosures and conflict of interest avoidance must apply to the review manager as well. Thank you for your feedback!
I understand what you are trying to do and there is a better way to do it. Anyone can participate in the reviews, and even individuals who are conflicted should still participate. In most cases they have considerable understanding of the domain and the library in particular, we want to hear what they have to say. However, the review manager should consider all of the above (1, 2, and 3) when evaluating the extent to which a particular reviewer should influence the outcome. For this, conspicuous disclosures are needed (see previous)
I think we both agree, my final proposal was not the points you mentioned (that are too stringent for boost context), but the sketch of the guidelines at the end that accounted for what you just said : *Clause for Reviewer:* *A Reviewer must not participate in reviewing
submissions in which (i) **They have contributed as an author or co-author, (ii) **They have a direct financial interest or contractual obligation tied to the submission. **Reviewers affiliated with the same organization as the author may participate, provided they disclose this relationship and their input is balanced by independent reviewers and reported at the discretion of the Review Manager.*
Ultimately I believe we just want *at least* to visualize if there is a meaningful correlation between two categorical variables: affiliations and acceptance. If the Reviewers gave their disclosure (that should be automatically required in the future, e.g., with an online Form), it should not be too complicated for the Review Manager to generate an automated report utilizing the Reviewers data table affiliation/acceptance. The report could compute the contingency table and draw a stacked bar chart or a mosaic plot or whatever, and the results be presented during the Review Manager report. Maybe transparency alone could reassure participants that the process is fair. If nonetheless a complaint was made during the Retrospective phase that Conflicts of Interest affected the neutrality of the review, the Review Wizards Council could i) acknowledge the complaint ii) investigate the conflict of interest iii) conduct an independent review iv) communicate findings and assign a new Review Manager. Best wishes,
On Wed, Oct 30, 2024 at 9:01 AM Klemens Morgenstern
Hi all,
thanks to everyone participating in the review! I really want to express my appreciation to the reviewers, especially those who invested multiple hours if not days to review this library: thank you Vinnie, Ruben, Mohammad and Marcelo.
I did not write a summary on purpose, because I didn't think criticizing a review was a good idea, as it might discourage others from participating in reviews. This is not new, summarizing the review & discussion has not always happened in the review decision. https://lists.boost.org/Archives/boost/2021/03/251099.php https://lists.boost.org/Archives/boost/2022/06/253193.php https://lists.boost.org/Archives/boost/2024/01/255717.php However, having this review called a "miscarriage" because I posted a conclusion after the (extended) review period ended, showed my approach to be misguided. I want to start by saying that "low-effort" in a review isn't a problem. I rather see it as an issue that people seem to expect themselves to invest hours into a review and if they can't, they don't write a review. I think reviews that are done quickly are fine if they state that. This is the reason that the outcome of a review is not determined by majority vote. However, a review should address the actual library and actual problems or clearly state that it's about a personal preference. Since I have received two reject reviews that I weighed on opposite sides of the spectrum, comparing them should be a useful exercise. The review summary I should have written when announcing the result can be found at the end. # Review Analysis ## Low weighed https://lists.boost.org/Archives/boost/2024/10/258159.php
My primary reasoning is that so far almost all the review discussion I've seen is focusing solely on the Asio aspect which makes this a miss.
The review subject is the library at question, not the discussion around it. What the review is actually referring to is essential for it's weighing.
[...] the actual meat and potatoes of the library [...] is supposed to be MQTT parsing and serialization and broker management.
This is an assertion without anything backing it up. It might hold true for HTTP 1.1, yet many protocols have pretty easy binary protocols, but rather difficult state management. There is no technical basis here, nor experience. The review later says "I don't have a lot of experience with MQTT". Such an assumption needs to be backed either by technical detail or by experience. Neither is present. Thirdly "broker management" gets mentioned. A broker is an MQTT server and this is a client library. Managing brokers in a client library requires IO.
what I'd like to see in the future for libraries like these is a first-class interface _without_ I/O.
This is the core of this review: a personal preference of how libraries like this have to be structured. It's not a review of the library submitted, but a rejection of the kind of library it is (an asio based client).
I don't have a lot of experience with MQTT as a protocol but my operating assumption is that if TLS/SSL can be made without I/O, so can anything else.
Just because something can be done doesn't mean it's the right or even a good idea. It could also turn an I/O less MQTT library into an unusable callback hell. The review states correctly that this is an assumption, and thus it cannot get weighed as highly as a discussion of an actual implementation. In Summary, this review rejects async-mqtt5 not on the library's own merit, but because it wants a fundamentally different approach to client libraries. A rejection based on kind - especially when multiple libraries like the submitted are already in boost - would need more than a minority opinion. Because of that the review gets weighed significantly lower, because that requires more reviews with the same opinion to be submitted. # Highly Weighed https://lists.boost.org/Archives/boost/2024/10/258190.php The other REJECT review got weighed significantly higher and I would like to highlight a few things that made it valuable.
It is useful for MQTT clients that are not resource constrained embedded systems.
First of all the review starts with a "it's useful for X" even though it later rejects it. This shows consideration of the general utility of the library and other users.
I immediately ran into this lack of visibility when I used EMQX as the broker to test asnyc-mqtt5 against. As I was eventually able to deduce, EMQX doesn't like a blank client name (...). At least this sent me digging through the code to see how it works, adding "logging" so I could see when messages were being built, sent, received. I've taken a fairly thorough look at the code in my review.
This is very helpful: the review explains a problem and suggests a solution (logging).
Id really like to see a more composed/layered approach used.
Within the context of the review it's obvious why this is desired to analyze the internal behaviour. It's neither ideological nor fictional. In a follow-up email the reviewer wrote this:
A high level library that can do all this needs a tunable "QoS" in the general sense (not just the selection of one of 3 values for an MQTT parameter). A low level library can legitimately claim its out of scope. This library has high level features but appears to lack some tunability (but that could be user ignorance on my part) and also access to low level features to allow the user to take responsibility for these issues. That needs addressing.
This also is very good review content, as it points out a problem but doesn't prescribe a specific solution. In a layered approach a user could tune this by accessing the layers, or it could be by exposing this functionality in the high level interface. This is very helpful because it clearly states a problem to be solved. The library author can use this as advise to improve his library (whether accepted or rejected) and the review manager can use those comments to form a decision or formulate conditions. I hope those two examples help future reviewers write impactful reviews. # Review Summary Overall there were two main worries shared by most reviews: lack of transparency and compile times. Additionally, a bulk publish and type erased connections were recommended by three reviews. Two REJECT recommendations: https://lists.boost.org/Archives/boost/2024/10/258159.php This review has little technical substance but promotes I/O less protocol libraries. It doesn't consider the MQTT protocol at any point, but just assumes it is the correct approach without technical basis or experience. https://lists.boost.org/Archives/boost/2024/10/258190.php The review criticizes the lack of fine-tuning and access to the internal workings of the client. It criticizes the lack of transparency, yet does consider the library useful. In a follow-up email the reviewer states it's a "reject at this point", meaning the reviewer might change it to an accept if the library moves the right direction. One ACCEPT recommendations: https://lists.boost.org/Archives/boost/2024/10/258172.php This review, with considerable asio experience, does take a detailed look at the implementation, sadly without building the library. The review criticizes property interface, but concludes the library overall is solid enough to be in boost. Three CONDITIONAL ACCEPT recommendations: https://lists.boost.org/Archives/boost/2024/10/258189.php The review points out some issues with the async_publish behaviour: it should be able to publish more than one message at a time and should not trigger write calls. The two conditions for acceptance are: 1. Add logging. 2. Add a separately compiled connection using any_completion_handler. NOTE: The reviewer did criticize the executor correctness prior to his review and mentioned it in a recommendation. https://lists.boost.org/Archives/boost/2024/10/258149.php This review was approving of a high-level library based on the reviewers prior work in the IoT world and his current work with asio. The review shares the compile time concerns and recommends evaluation of a type-erased stream. It has three conditions for acceptance: 1. Debugging easier (e.g. logging) 2. Executor correctness 3. TLS/websocket shutdown https://lists.boost.org/Archives/boost/2024/10/258164.php The review likewise approves the high-level interface based on the reviewer's prior experience with mqtt_cpp. It likewise recommends evaluating a type-erased stream. It has the following criteria for acceptance: 1. Error reporting 2. Break the hard dependency on Beast & TLS 3. Limits on internal queues 4. Sanitizer builds. In Summary: 2 REJECT 1 ACCEPT 3 CONDITIONAL ACCEPT The highly weighed reject review was declared "reject at this point" by its author, i.e. not a categorical rejection. That means that only one rejection is based on the fundamental idea of what the library is, i.e. 5/6 reviews generally agree that an asio based mqtt5 client library could or should be accepted into boost. Among the accepting reviews are three from authors of similar libraries (Beast, Redis, MySql) and one the maintainer of Beast. One has professional IoT experience, another direct experience with mqtt. Therefore a CONDITIONAL ACCEPT is the correct conclusion of this review. Regarding the conditions: The logging/error issue was a condition of all three conditional reviews and mentioned as a possible solution by one rejecting review. The executor correctness is only a condition in one of the reviews, but considered a bug by others during the discussion. Since this is a violation of being "Fully Boost.Asio compliant" as mentioned in the docs of async-mqtt5 it needs to be fixed.
Maybe you should have recused yourself from this review, considering you have previous ties to the author that may have made you quite biased against reject reviews, like calling them low effort without having any idea how much effort was put into them. Robert Ramey did recuse himself from reviewing the other library precisely because he knew the author and he didn’t want to conduct a questionable review. I’m not saying that knowing an author is a problem, but it does look like this library was already accepted before the review started. Claudio On Thu, 31 Oct 2024 at 04:41, Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
On Wed, Oct 30, 2024 at 9:01 AM Klemens Morgenstern
wrote: Hi all,
thanks to everyone participating in the review! I really want to express my appreciation to the reviewers, especially those who invested multiple hours if not days to review this library: thank you Vinnie, Ruben, Mohammad and Marcelo.
I did not write a summary on purpose, because I didn't think criticizing a review was a good idea, as it might discourage others from participating in reviews.
This is not new, summarizing the review & discussion has not always happened in the review decision.
https://lists.boost.org/Archives/boost/2021/03/251099.php https://lists.boost.org/Archives/boost/2022/06/253193.php https://lists.boost.org/Archives/boost/2024/01/255717.php
However, having this review called a "miscarriage" because I posted a conclusion after the (extended) review period ended, showed my approach to be misguided.
I want to start by saying that "low-effort" in a review isn't a problem. I rather see it as an issue that people seem to expect themselves to invest hours into a review and if they can't, they don't write a review. I think reviews that are done quickly are fine if they state that. This is the reason that the outcome of a review is not determined by majority vote.
However, a review should address the actual library and actual problems or clearly state that it's about a personal preference.
Since I have received two reject reviews that I weighed on opposite sides of the spectrum, comparing them should be a useful exercise.
The review summary I should have written when announcing the result can be found at the end.
# Review Analysis
## Low weighed https://lists.boost.org/Archives/boost/2024/10/258159.php
My primary reasoning is that so far almost all the review discussion I've seen is focusing solely on the Asio aspect which makes this a miss.
The review subject is the library at question, not the discussion around it. What the review is actually referring to is essential for it's weighing.
[...] the actual meat and potatoes of the library [...] is supposed to be MQTT parsing and serialization and broker management.
This is an assertion without anything backing it up. It might hold true for HTTP 1.1, yet many protocols have pretty easy binary protocols, but rather difficult state management. There is no technical basis here, nor experience. The review later says "I don't have a lot of experience with MQTT".
Such an assumption needs to be backed either by technical detail or by experience. Neither is present.
Thirdly "broker management" gets mentioned. A broker is an MQTT server and this is a client library. Managing brokers in a client library requires IO.
what I'd like to see in the future for libraries like these is a first-class interface _without_ I/O.
This is the core of this review: a personal preference of how libraries like this have to be structured. It's not a review of the library submitted, but a rejection of the kind of library it is (an asio based client).
I don't have a lot of experience with MQTT as a protocol but my operating assumption is that if TLS/SSL can be made without I/O, so can anything else.
Just because something can be done doesn't mean it's the right or even a good idea. It could also turn an I/O less MQTT library into an unusable callback hell. The review states correctly that this is an assumption, and thus it cannot get weighed as highly as a discussion of an actual implementation.
In Summary, this review rejects async-mqtt5 not on the library's own merit, but because it wants a fundamentally different approach to client libraries. A rejection based on kind - especially when multiple libraries like the submitted are already in boost - would need more than a minority opinion. Because of that the review gets weighed significantly lower, because that requires more reviews with the same opinion to be submitted.
# Highly Weighed
https://lists.boost.org/Archives/boost/2024/10/258190.php
The other REJECT review got weighed significantly higher and I would like to highlight a few things that made it valuable.
It is useful for MQTT clients that are not resource constrained embedded systems.
First of all the review starts with a "it's useful for X" even though it later rejects it. This shows consideration of the general utility of the library and other users.
I immediately ran into this lack of visibility when I used EMQX as the broker to test asnyc-mqtt5 against. As I was eventually able to deduce, EMQX doesn't like a blank client name (...). At least this sent me digging through the code to see how it works, adding "logging" so I could see when messages were being built, sent, received. I've taken a fairly thorough look at the code in my review.
This is very helpful: the review explains a problem and suggests a solution (logging).
Id really like to see a more composed/layered approach used.
Within the context of the review it's obvious why this is desired to analyze the internal behaviour. It's neither ideological nor fictional. In a follow-up email the reviewer wrote this:
A high level library that can do all this needs a tunable "QoS" in the general sense (not just the selection of one of 3 values for an MQTT parameter). A low level library can legitimately claim its out of scope. This library has high level features but appears to lack some tunability (but that could be user ignorance on my part) and also access to low level features to allow the user to take responsibility for these issues. That needs addressing.
This also is very good review content, as it points out a problem but doesn't prescribe a specific solution. In a layered approach a user could tune this by accessing the layers, or it could be by exposing this functionality in the high level interface. This is very helpful because it clearly states a problem to be solved. The library author can use this as advise to improve his library (whether accepted or rejected) and the review manager can use those comments to form a decision or formulate conditions.
I hope those two examples help future reviewers write impactful reviews.
# Review Summary
Overall there were two main worries shared by most reviews: lack of transparency and compile times. Additionally, a bulk publish and type erased connections were recommended by three reviews.
Two REJECT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258159.php This review has little technical substance but promotes I/O less protocol libraries. It doesn't consider the MQTT protocol at any point, but just assumes it is the correct approach without technical basis or experience.
https://lists.boost.org/Archives/boost/2024/10/258190.php The review criticizes the lack of fine-tuning and access to the internal workings of the client. It criticizes the lack of transparency, yet does consider the library useful. In a follow-up email the reviewer states it's a "reject at this point", meaning the reviewer might change it to an accept if the library moves the right direction.
One ACCEPT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258172.php This review, with considerable asio experience, does take a detailed look at the implementation, sadly without building the library. The review criticizes property interface, but concludes the library overall is solid enough to be in boost.
Three CONDITIONAL ACCEPT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258189.php The review points out some issues with the async_publish behaviour: it should be able to publish more than one message at a time and should not trigger write calls. The two conditions for acceptance are:
1. Add logging. 2. Add a separately compiled connection using any_completion_handler.
NOTE: The reviewer did criticize the executor correctness prior to his review and mentioned it in a recommendation.
https://lists.boost.org/Archives/boost/2024/10/258149.php This review was approving of a high-level library based on the reviewers prior work in the IoT world and his current work with asio. The review shares the compile time concerns and recommends evaluation of a type-erased stream. It has three conditions for acceptance:
1. Debugging easier (e.g. logging) 2. Executor correctness 3. TLS/websocket shutdown
https://lists.boost.org/Archives/boost/2024/10/258164.php The review likewise approves the high-level interface based on the reviewer's prior experience with mqtt_cpp. It likewise recommends evaluating a type-erased stream.
It has the following criteria for acceptance:
1. Error reporting 2. Break the hard dependency on Beast & TLS 3. Limits on internal queues 4. Sanitizer builds.
In Summary:
2 REJECT 1 ACCEPT 3 CONDITIONAL ACCEPT
The highly weighed reject review was declared "reject at this point" by its author, i.e. not a categorical rejection. That means that only one rejection is based on the fundamental idea of what the library is, i.e. 5/6 reviews generally agree that an asio based mqtt5 client library could or should be accepted into boost.
Among the accepting reviews are three from authors of similar libraries (Beast, Redis, MySql) and one the maintainer of Beast. One has professional IoT experience, another direct experience with mqtt.
Therefore a CONDITIONAL ACCEPT is the correct conclusion of this review.
Regarding the conditions:
The logging/error issue was a condition of all three conditional reviews and mentioned as a possible solution by one rejecting review. The executor correctness is only a condition in one of the reviews, but considered a bug by others during the discussion. Since this is a violation of being "Fully Boost.Asio compliant" as mentioned in the docs of async-mqtt5 it needs to be fixed.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Thu, Oct 31, 2024 at 2:20 PM Claudio DeSouza
Maybe you should have recused yourself from this review, considering you have previous ties to the author that may have made you quite biased against reject reviews, like calling them low effort without having any idea how much effort was put into them.
Robert Ramey did recuse himself from reviewing the other library precisely because he knew the author and he didn’t want to conduct a questionable review. I’m not saying that knowing an author is a problem, but it does look like this library was already accepted before the review started.
Claudio
On Thu, 31 Oct 2024 at 04:41, Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
On Wed, Oct 30, 2024 at 9:01 AM Klemens Morgenstern
wrote: Hi all,
thanks to everyone participating in the review! I really want to express my appreciation to the reviewers, especially those who invested multiple hours if not days to review this library: thank you Vinnie, Ruben, Mohammad and Marcelo.
I did not write a summary on purpose, because I didn't think criticizing a review was a good idea, as it might discourage others from participating in reviews.
This is not new, summarizing the review & discussion has not always happened in the review decision.
https://lists.boost.org/Archives/boost/2021/03/251099.php https://lists.boost.org/Archives/boost/2022/06/253193.php https://lists.boost.org/Archives/boost/2024/01/255717.php
However, having this review called a "miscarriage" because I posted a conclusion after the (extended) review period ended, showed my approach to be misguided.
I want to start by saying that "low-effort" in a review isn't a problem. I rather see it as an issue that people seem to expect themselves to invest hours into a review and if they can't, they don't write a review. I think reviews that are done quickly are fine if they state that. This is the reason that the outcome of a review is not determined by majority vote.
However, a review should address the actual library and actual problems or clearly state that it's about a personal preference.
Since I have received two reject reviews that I weighed on opposite sides of the spectrum, comparing them should be a useful exercise.
The review summary I should have written when announcing the result can be found at the end.
# Review Analysis
## Low weighed https://lists.boost.org/Archives/boost/2024/10/258159.php
My primary reasoning is that so far almost all the review discussion I've seen is focusing solely on the Asio aspect which makes this a miss.
The review subject is the library at question, not the discussion around it. What the review is actually referring to is essential for it's weighing.
[...] the actual meat and potatoes of the library [...] is supposed to be MQTT parsing and serialization and broker management.
This is an assertion without anything backing it up. It might hold true for HTTP 1.1, yet many protocols have pretty easy binary protocols, but rather difficult state management. There is no technical basis here, nor experience. The review later says "I don't have a lot of experience with MQTT".
Such an assumption needs to be backed either by technical detail or by experience. Neither is present.
Thirdly "broker management" gets mentioned. A broker is an MQTT server and this is a client library. Managing brokers in a client library requires IO.
what I'd like to see in the future for libraries like these is a first-class interface _without_ I/O.
This is the core of this review: a personal preference of how libraries like this have to be structured. It's not a review of the library submitted, but a rejection of the kind of library it is (an asio based client).
I don't have a lot of experience with MQTT as a protocol but my operating assumption is that if TLS/SSL can be made without I/O, so can anything else.
Just because something can be done doesn't mean it's the right or even a good idea. It could also turn an I/O less MQTT library into an unusable callback hell. The review states correctly that this is an assumption, and thus it cannot get weighed as highly as a discussion of an actual implementation.
In Summary, this review rejects async-mqtt5 not on the library's own merit, but because it wants a fundamentally different approach to client libraries. A rejection based on kind - especially when multiple libraries like the submitted are already in boost - would need more than a minority opinion. Because of that the review gets weighed significantly lower, because that requires more reviews with the same opinion to be submitted.
# Highly Weighed
https://lists.boost.org/Archives/boost/2024/10/258190.php
The other REJECT review got weighed significantly higher and I would like to highlight a few things that made it valuable.
It is useful for MQTT clients that are not resource constrained embedded systems.
First of all the review starts with a "it's useful for X" even though it later rejects it. This shows consideration of the general utility of the library and other users.
I immediately ran into this lack of visibility when I used EMQX as the broker to test asnyc-mqtt5 against. As I was eventually able to deduce, EMQX doesn't like a blank client name (...). At least this sent me digging through the code to see how it works, adding "logging" so I could see when messages were being built, sent, received. I've taken a fairly thorough look at the code in my review.
This is very helpful: the review explains a problem and suggests a solution (logging).
Id really like to see a more composed/layered approach used.
Within the context of the review it's obvious why this is desired to analyze the internal behaviour. It's neither ideological nor fictional. In a follow-up email the reviewer wrote this:
A high level library that can do all this needs a tunable "QoS" in the general sense (not just the selection of one of 3 values for an MQTT parameter). A low level library can legitimately claim its out of scope. This library has high level features but appears to lack some tunability (but that could be user ignorance on my part) and also access to low level features to allow the user to take responsibility for these issues. That needs addressing.
This also is very good review content, as it points out a problem but doesn't prescribe a specific solution. In a layered approach a user could tune this by accessing the layers, or it could be by exposing this functionality in the high level interface. This is very helpful because it clearly states a problem to be solved. The library author can use this as advise to improve his library (whether accepted or rejected) and the review manager can use those comments to form a decision or formulate conditions.
I hope those two examples help future reviewers write impactful reviews.
# Review Summary
Overall there were two main worries shared by most reviews: lack of transparency and compile times. Additionally, a bulk publish and type erased connections were recommended by three reviews.
Two REJECT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258159.php This review has little technical substance but promotes I/O less protocol libraries. It doesn't consider the MQTT protocol at any point, but just assumes it is the correct approach without technical basis or experience.
https://lists.boost.org/Archives/boost/2024/10/258190.php The review criticizes the lack of fine-tuning and access to the internal workings of the client. It criticizes the lack of transparency, yet does consider the library useful. In a follow-up email the reviewer states it's a "reject at this point", meaning the reviewer might change it to an accept if the library moves the right direction.
One ACCEPT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258172.php This review, with considerable asio experience, does take a detailed look at the implementation, sadly without building the library. The review criticizes property interface, but concludes the library overall is solid enough to be in boost.
Three CONDITIONAL ACCEPT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258189.php The review points out some issues with the async_publish behaviour: it should be able to publish more than one message at a time and should not trigger write calls. The two conditions for acceptance are:
1. Add logging. 2. Add a separately compiled connection using any_completion_handler.
NOTE: The reviewer did criticize the executor correctness prior to his review and mentioned it in a recommendation.
https://lists.boost.org/Archives/boost/2024/10/258149.php This review was approving of a high-level library based on the reviewers prior work in the IoT world and his current work with asio. The review shares the compile time concerns and recommends evaluation of a type-erased stream. It has three conditions for acceptance:
1. Debugging easier (e.g. logging) 2. Executor correctness 3. TLS/websocket shutdown
https://lists.boost.org/Archives/boost/2024/10/258164.php The review likewise approves the high-level interface based on the reviewer's prior experience with mqtt_cpp. It likewise recommends evaluating a type-erased stream.
It has the following criteria for acceptance:
1. Error reporting 2. Break the hard dependency on Beast & TLS 3. Limits on internal queues 4. Sanitizer builds.
In Summary:
2 REJECT 1 ACCEPT 3 CONDITIONAL ACCEPT
The highly weighed reject review was declared "reject at this point" by its author, i.e. not a categorical rejection. That means that only one rejection is based on the fundamental idea of what the library is, i.e. 5/6 reviews generally agree that an asio based mqtt5 client library could or should be accepted into boost.
Among the accepting reviews are three from authors of similar libraries (Beast, Redis, MySql) and one the maintainer of Beast. One has professional IoT experience, another direct experience with mqtt.
Therefore a CONDITIONAL ACCEPT is the correct conclusion of this review.
Regarding the conditions:
The logging/error issue was a condition of all three conditional reviews and mentioned as a possible solution by one rejecting review. The executor correctness is only a condition in one of the reviews, but considered a bug by others during the discussion. Since this is a violation of being "Fully Boost.Asio compliant" as mentioned in the docs of async-mqtt5 it needs to be fixed.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Thu, Oct 31, 2024, 11:40 PM Claudio DeSouza via Boost < boost@lists.boost.org> wrote:
Maybe you should have recused yourself from this review, considering you have previous ties to the author that may have made you quite biased against reject reviews, like calling them low effort without having any idea how much effort was put into them.
I have no previous ties with the author. The first time I heard of them was when they asked me to be the review manager for their library. I do not appreciate this kind of slander. Robert Ramey did recuse himself from reviewing the other library precisely
because he knew the author and he didn’t want to conduct a questionable review. I’m not saying that knowing an author is a problem, but it does look like this library was already accepted before the review started.
Claudio
On Thu, 31 Oct 2024 at 04:41, Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
On Wed, Oct 30, 2024 at 9:01 AM Klemens Morgenstern
wrote: Hi all,
thanks to everyone participating in the review! I really want to express my appreciation to the reviewers, especially those who invested multiple hours if not days to review this library: thank you Vinnie, Ruben, Mohammad and Marcelo.
I did not write a summary on purpose, because I didn't think criticizing a review was a good idea, as it might discourage others from participating in reviews.
This is not new, summarizing the review & discussion has not always happened in the review decision.
https://lists.boost.org/Archives/boost/2021/03/251099.php https://lists.boost.org/Archives/boost/2022/06/253193.php https://lists.boost.org/Archives/boost/2024/01/255717.php
However, having this review called a "miscarriage" because I posted a conclusion after the (extended) review period ended, showed my approach to be misguided.
I want to start by saying that "low-effort" in a review isn't a problem. I rather see it as an issue that people seem to expect themselves to invest hours into a review and if they can't, they don't write a review. I think reviews that are done quickly are fine if they state that. This is the reason that the outcome of a review is not determined by majority vote.
However, a review should address the actual library and actual problems or clearly state that it's about a personal preference.
Since I have received two reject reviews that I weighed on opposite sides of the spectrum, comparing them should be a useful exercise.
The review summary I should have written when announcing the result can be found at the end.
# Review Analysis
## Low weighed https://lists.boost.org/Archives/boost/2024/10/258159.php
My primary reasoning is that so far almost all the review discussion I've seen is focusing solely on the Asio aspect which makes this a miss.
The review subject is the library at question, not the discussion around it. What the review is actually referring to is essential for it's weighing.
[...] the actual meat and potatoes of the library [...] is supposed to be MQTT parsing and serialization and broker management.
This is an assertion without anything backing it up. It might hold true for HTTP 1.1, yet many protocols have pretty easy binary protocols, but rather difficult state management. There is no technical basis here, nor experience. The review later says "I don't have a lot of experience with MQTT".
Such an assumption needs to be backed either by technical detail or by experience. Neither is present.
Thirdly "broker management" gets mentioned. A broker is an MQTT server and this is a client library. Managing brokers in a client library requires IO.
what I'd like to see in the future for libraries like these is a first-class interface _without_ I/O.
This is the core of this review: a personal preference of how libraries like this have to be structured. It's not a review of the library submitted, but a rejection of the kind of library it is (an asio based client).
I don't have a lot of experience with MQTT as a protocol but my operating assumption is that if TLS/SSL can be made without I/O, so can anything else.
Just because something can be done doesn't mean it's the right or even a good idea. It could also turn an I/O less MQTT library into an unusable callback hell. The review states correctly that this is an assumption, and thus it cannot get weighed as highly as a discussion of an actual implementation.
In Summary, this review rejects async-mqtt5 not on the library's own merit, but because it wants a fundamentally different approach to client libraries. A rejection based on kind - especially when multiple libraries like the submitted are already in boost - would need more than a minority opinion. Because of that the review gets weighed significantly lower, because that requires more reviews with the same opinion to be submitted.
# Highly Weighed
https://lists.boost.org/Archives/boost/2024/10/258190.php
The other REJECT review got weighed significantly higher and I would like to highlight a few things that made it valuable.
It is useful for MQTT clients that are not resource constrained embedded systems.
First of all the review starts with a "it's useful for X" even though it later rejects it. This shows consideration of the general utility of the library and other users.
I immediately ran into this lack of visibility when I used EMQX as the broker to test asnyc-mqtt5 against. As I was eventually able to deduce, EMQX doesn't like a blank client name (...). At least this sent me digging through the code to see how it works, adding "logging" so I could see when messages were being built, sent, received. I've taken a fairly thorough look at the code in my review.
This is very helpful: the review explains a problem and suggests a solution (logging).
Id really like to see a more composed/layered approach used.
Within the context of the review it's obvious why this is desired to analyze the internal behaviour. It's neither ideological nor fictional. In a follow-up email the reviewer wrote this:
A high level library that can do all this needs a tunable "QoS" in the general sense (not just the selection of one of 3 values for an MQTT parameter). A low level library can legitimately claim its out of scope. This library has high level features but appears to lack some tunability (but that could be user ignorance on my part) and also access to low level features to allow the user to take responsibility for these issues. That needs addressing.
This also is very good review content, as it points out a problem but doesn't prescribe a specific solution. In a layered approach a user could tune this by accessing the layers, or it could be by exposing this functionality in the high level interface. This is very helpful because it clearly states a problem to be solved. The library author can use this as advise to improve his library (whether accepted or rejected) and the review manager can use those comments to form a decision or formulate conditions.
I hope those two examples help future reviewers write impactful reviews.
# Review Summary
Overall there were two main worries shared by most reviews: lack of transparency and compile times. Additionally, a bulk publish and type erased connections were recommended by three reviews.
Two REJECT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258159.php This review has little technical substance but promotes I/O less protocol libraries. It doesn't consider the MQTT protocol at any point, but just assumes it is the correct approach without technical basis or experience.
https://lists.boost.org/Archives/boost/2024/10/258190.php The review criticizes the lack of fine-tuning and access to the internal workings of the client. It criticizes the lack of transparency, yet does consider the library useful. In a follow-up email the reviewer states it's a "reject at this point", meaning the reviewer might change it to an accept if the library moves the right direction.
One ACCEPT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258172.php This review, with considerable asio experience, does take a detailed look at the implementation, sadly without building the library. The review criticizes property interface, but concludes the library overall is solid enough to be in boost.
Three CONDITIONAL ACCEPT recommendations:
https://lists.boost.org/Archives/boost/2024/10/258189.php The review points out some issues with the async_publish behaviour: it should be able to publish more than one message at a time and should not trigger write calls. The two conditions for acceptance are:
1. Add logging. 2. Add a separately compiled connection using any_completion_handler.
NOTE: The reviewer did criticize the executor correctness prior to his review and mentioned it in a recommendation.
https://lists.boost.org/Archives/boost/2024/10/258149.php This review was approving of a high-level library based on the reviewers prior work in the IoT world and his current work with asio. The review shares the compile time concerns and recommends evaluation of a type-erased stream. It has three conditions for acceptance:
1. Debugging easier (e.g. logging) 2. Executor correctness 3. TLS/websocket shutdown
https://lists.boost.org/Archives/boost/2024/10/258164.php The review likewise approves the high-level interface based on the reviewer's prior experience with mqtt_cpp. It likewise recommends evaluating a type-erased stream.
It has the following criteria for acceptance:
1. Error reporting 2. Break the hard dependency on Beast & TLS 3. Limits on internal queues 4. Sanitizer builds.
In Summary:
2 REJECT 1 ACCEPT 3 CONDITIONAL ACCEPT
The highly weighed reject review was declared "reject at this point" by its author, i.e. not a categorical rejection. That means that only one rejection is based on the fundamental idea of what the library is, i.e. 5/6 reviews generally agree that an asio based mqtt5 client library could or should be accepted into boost.
Among the accepting reviews are three from authors of similar libraries (Beast, Redis, MySql) and one the maintainer of Beast. One has professional IoT experience, another direct experience with mqtt.
Therefore a CONDITIONAL ACCEPT is the correct conclusion of this review.
Regarding the conditions:
The logging/error issue was a condition of all three conditional reviews and mentioned as a possible solution by one rejecting review. The executor correctness is only a condition in one of the reviews, but considered a bug by others during the discussion. Since this is a violation of being "Fully Boost.Asio compliant" as mentioned in the docs of async-mqtt5 it needs to be fixed.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
To clarify
I've been approached by the mireo authors and have volunteered to be their review manager.
And:
I only volunteered for the mireo library (i.e. the "other" one from your perspective). I don't think redboltz is ready for review so I don't want to be RM.
Furthermore I fear I might be too biased between the two of the libraries to be review manager of both.
So there was some sort of contact between you and the authors and you made you've stated enough to indicate you wanted this library in boost even before the review started, including by indicating you had a bias to review one, but not the other. It's very clear this review has not gone up to standard. Claudio
On Thu, 31 Oct 2024, 17:06 [soda-pressed] via Boost,
So there was some sort of contact between you and the authors (...)
Unless you want to present actual proofs of any misconduct, I, as an individual member of Boost Community, would suggest we stop there, before we reach unacceptable levels of ridiculousness in this discussion. Best regards, Mateusz
Alright, seems like no one else has any strong feelings. I withdraw my previous comments about the review manager and verdict, and yield to the process. I suppose then that congratulations are in order for the newly accepted library. Let's clear ourselves of this mess and welcome a new Boost author. - Christian
On 1 Nov 2024, at 17:39, Christian Mazakas via Boost
wrote: Alright, seems like no one else has any strong feelings.
I would like to register opposition, for the future, to one of the arguments which was advanced by the RM: that a recommendation to reject, based supposedly on the feeling that "the library should be something else" can be ignored or discounted. I dont know enough about the particulars in this case, maybe reasonable in this case, but I dont want to hear this argument in the future again. If the reviewer is expressing that the design is flawed, that should be weighted on its technical merits. Cheers, Kostas
On Mon, Nov 4, 2024 at 1:40 PM Kostas Savvidis via Boost < boost@lists.boost.org> wrote:
If the reviewer is expressing that the design is flawed, that should be weighted on its technical merits.
I agree, visible design flaws should be well articulated by reviewers and accounted for by the review manager. However, that is not what happened here. Instead, the review made this equivalent statement: "The library is designed a certain way, and it works. However it would be better if the library was designed this different way." This may be true, and based on conversations with other contributors who have been here longer than me I don't think it is a strong reason (on its own) to reject a library. I think there would be value in having a more formal explanation of the criteria used for acceptance. I say this without first checking to see if we have already written something up on the website (Turcan?) It was explained to me that a library should be accepted if Boost is better off with the library than without it, and I think it was implied that the library meets or exceeds the level of quality expected of its interface, implementation, and documentation. Does the submission in question meet these requirements, despite not implementing the "better design?" This is the question that I think should be answered. Thanks
On Mon, Nov 4, 2024, 23:11 Vinnie Falco via Boost
On Mon, Nov 4, 2024 at 1:40 PM Kostas Savvidis via Boost < boost@lists.boost.org> wrote:
"The library is designed a certain way, and it works. However it would be better if the library was designed this different way."
This may be true, and based on conversations with other contributors who have been here longer than me I don't think it is a strong reason (on its own) to reject a library.
Isn't criticizing the design of a library the whole point of the review process? The design may be "good enough" and it might "work", but is this the standard Boost is aspiring to? Should all comments about the overarching design be automatically dismissed and focus shifted solely on reviewing function signatures? Given the rationale like this: *This is the core of this review: a personal preference of how* *libraries like this have to be structured.* *It's not a review of the library submitted, but a rejection of the* *kind of library it is (an asio based client).* it seems like this is the case, and you cannot disagree with "the kind of the library it is". I completely agree with Kostas here.
On Mon, Nov 4, 2024 at 9:39 PM Thomas Fowlery
The design may be "good enough" and it might "work", but is this the standard Boost is aspiring to?
I wondered about this myself, back in March of 2024 when I posted this to the mailing list: https://lists.boost.org/Archives/boost//2024/03/256333.php However I have been informed that the criteria for accepting a library into Boost something more along these lines: * That the library is useful, and * Boost is better off with the library than without This would appear to be at odds with your implication that Boost submissions should aspire to more. Klemens position has been consistent and he stated it as a reply all the way back in March: https://lists.boost.org/Archives/boost/2024/03/256335.php He is technically not wrong, although based on the conversations after the review result for async-mqtt5 was posted it seems that there are differences of opinion as to what is the criteria for evaluating whether a library should be part of Boost or not. Interestingly my expectation that my post from March would stimulate a robust discussion about acceptance criteria was never satisfied, and perhaps this is the opportunity to catalyze that discussion again. What do you think are the qualities that should be found in a library submission during a formal review which would make it suitable to become part of Boost? Happy to hear from you Thomas, and everyone else. Thanks
Vinnie Falco wrote:
However I have been informed that the criteria for accepting a library into Boost something more along these lines:
* That the library is useful, and * Boost is better off with the library than without
The second bullet should be * Boost is better off with the library than without, _in the long term_ That is, it's fine to object to acceptance on the basis that this will prevent a better library being accepted in the future. Of course, this argument is based on speculation and hypotheticals, and as such does not carry as much weight as arguments based on the here and now, or arguments rooted in the specific and concrete. Also, there are several more implicit bullets that basically say that the library should be defect-free in implementation and usability, that it has adequate documentation, that it actually builds on popular compilers, that it has enough test coverage, that the design is in line with best $CURRENT_YEAR practices, etc etc
On Tue, Nov 5, 2024 at 5:41 AM Kostas Savvidis via Boost
On 1 Nov 2024, at 17:39, Christian Mazakas via Boost
wrote: Alright, seems like no one else has any strong feelings.
I would like to register opposition, for the future, to one of the arguments which was advanced by the RM: that a recommendation to reject, based supposedly on the feeling that "the library should be something else" can be ignored or discounted. I dont know enough about the particulars in this case, maybe reasonable in this case, but I dont want to hear this argument in the future again. If the reviewer is expressing that the design is flawed, that should be weighted on its technical merits.
How would you evaluate the technical merits of such a review?
Cheers, Kostas
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Mon, Nov 4, 2024 at 3:11 PM Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
How would you evaluate the technical merits of such a review?
I'd ask myself if the person doing the review had joined the club of Asio implementers like Niall and I. Interestingly, Niall and I had a wonderful discussion about "reimplementing Asio" (should be the name of the thread) but kind of surprisingly to me, none of the resident Asio fans actually chimed in. But back to it, the thing is, other people on the list had actually stepped in and said that, "Hey, maybe a protocol lib is a good idea and worth talking about". Thomas Fowlery above noted:
Meanwhile, the two rejections (and some non-review comments) brought up major design questions which haven't been addressed by either the review manager or the author(s). These facts make the outcome way less clear, in my opinion.
This was a bad Boost review because the author of the proposed library chose to ignore interacting with any and all "negative" discussion and the review manager did the exact same. There was no fostered discussion by the people who should've been fostering it. It mostly focused on me for dismissing a library for not hitting the level of "interesting" that I use when evaluating a library for inclusion into Boost. I think what makes Boost so great is that we all have our own ideas of what makes a library a Boost library. In this case, I think just implementing a protocol in Asio isn't interesting enough to warrant being a Boost library. Instead, I think a pure protocol library for MQTT is quite interesting and would be worthy of the Boost moniker. But what I think as an individual doesn't really matter. The point of Boost and review is to foster discussion and toss ideas around about how feasible it is to implement things. In this case, my impression is that because the library was an Asio library and because we like Asio, we'll just accept it. And that's basically exactly what happened. - Christian
On Tue, Nov 5, 2024 at 7:46 AM Christian Mazakas via Boost
On Mon, Nov 4, 2024 at 3:11 PM Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
How would you evaluate the technical merits of such a review?
This was a bad Boost review because the author of the proposed library chose to ignore interacting with any and all "negative" discussion and the review manager did the exact same.
IMO, a review manager is not supposed to participate in the discussion, except to ask for clarifications on issues that might be relevant to the review. In this case, this wasn't necessary as other participants did so. The author not being responsive enough should be brought up during the review period and not afterwards.
But what I think as an individual doesn't really matter. The point of Boost and review is to foster discussion and toss ideas around about how feasible it is to implement things.
I don't think this is correct. The point of the review is to evaluate the library under review. The discussion should happen before that, which is why the review gets announced. In this case, the intent to review was announced in May (or June). Discussion around this library also took place since then on Slack. The idea that the review period is meant to toss ideas around seems misguided to me. We should do a better job having these kinds of discussions before the review, rather than having these fruitless ones after.
On Wed, Oct 30, 2024 at 9:41 PM Klemens Morgenstern via Boost < boost@lists.boost.org> wrote:
I did not write a summary on purpose, because I didn't think criticizing a review was a good idea, as it might discourage others from participating in reviews.
... Yeah, that's cool and all but I never actually expected anyone but me to understand the importance of protocol libraries. This is more about the author deliberately dodging any and all negative criticism of the library, even when other Boost authors explicitly say these issues are worth talking about. This review was not handled at the quality a Boost review should be at. Relevant discussions were not had and then what compounds this is that the review manager just then does an unqualified acceptance of the library with no rationale. - Christian
Please be aware that it is entirely the responsibility of the review manager to conduct the review the best way they see it. It doesn't help the community if we second guess the deliberations that review managers undertake without complete information. Everyone should have some patience. Klemens, thank you for the detailed evaluation "summary". -- -- René Ferdinand Rivera Morell -- Don't Assume Anything -- No Supone Nada -- Robot Dreams - http://robot-dreams.net
participants (15)
-
[soda-pressed]
-
Andrey Semashev
-
Arnaud Becheler
-
Christian Mazakas
-
Claudio DeSouza
-
Darryl Green
-
Glen Fernandes
-
Ivica Siladic
-
Klemens Morgenstern
-
Kostas Savvidis
-
Mateusz Loskot
-
Peter Dimov
-
René Ferdinand Rivera Morell
-
Thomas Fowlery
-
Vinnie Falco