Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)

Subject: Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)
From: Daniela Engert (dani_at_[hidden])
Date: 2018-01-28 16:10:02

Hi folks,

this is my review of Boost.Outcome v2.

tl;dr
The library should be *accepted*

Am 18.01.2018 um 23:07 schrieb charleyb123 . via Boost:

> - What is your evaluation of the design?

It is clean and matches my expectations. In particular, I like the
defaults and the ease-of-use without annoying ceremony to construct
instances of result<> or outcome<>. As far as I am concerned this
matches my typical use-cases. In this regard I prefer Outcome over the
WG21-proposed Expected. The same is true for the chosen design of the
observers.

> - What is your evaluation of the implementation?

I didn't dig too deep into the current source code, but afaics the
sources are approachable and not too convoluted. Even I can understand
what's going on there.

> - What is your evaluation of the documentation?

Congratulations, I love it. There are some minor inconsistencies between
code snippets and their textual explanation. Therefore I recommend
another round of proof-reading by someone who is not involved in the
development process, therefore witnessing those warts much easier not
being spoilt by a tainted mind.

> - What is your evaluation of the potential usefulness of the library?

Our team has use-cases for such kind of library just as laid out in the
documentation. Therefore I'll happily pick Boost.Outcome for that. My
gut feeling is that we are not alone in this regard.

> - Did you try to use the library? With what compiler? Did you have any
> problems?

I cloned the library locally and integrated it into Boost 1.66.0 as
described in the documentation. Using this augmented Boost 1.66.0, I ran
all of my tests and the evaluation with Visual Studio 2017 Update 5
(i.e. toolset 19.12). As already pointed out, the compiler from toolset
19.12 will ice in such a scenario. Therefore I upgraded the toolset to
version 19.13.26122 (the latest daily toolset version as of today) and
configured my VS test solutions and the commandline environment to
pick-up this toolset version.

With this setup in place, I first ran the test-suite which passed
without failures or compiler warnings at /W4 /std:c++latest
/permissive-. Great! Then I compiled and ran the code snippets from the
documentation subdirectory while studying the tutorial. Those are fine
as well. Beyond that I plugged result<> into our own sources where we
use a similar but much less sophisticated class for the same purpose.
This turned out to work well, too.

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

I spent a couple of hours over the course of the last week on the review
process.

> - Are you knowledgeable about the problem domain?

Somewhat. We already use a similar class in our customer projects where
it fits the needs.

> - Do you think the library should be accepted as a Boost library?

*yes* without conditions.

Ciao
  Dani