Re: [boost] [Boost-users] [process] Formal review results

Subject: Re: [boost] [Boost-users] [process] Formal review results
From: Antony Polukhin (antoshkka_at_[hidden])
Date: 2016-11-13 11:03:14

2016-11-13 4:35 GMT-08:00 Bjorn Reese <breese_at_[hidden]>:
> On 11/09/2016 07:45 PM, Antony Polukhin wrote:
>>
>> The Boost.Process library is accepted.
>
>
> Unconditionally?

Conditional acceptance means that the mini-review is required. I can
see no issues that require fixing in mini-review:
* Currently there's no consensus on how to improve the arguments
passing, so a mini review for changed interface would not improve the
consensus.
* There's no big unsolvable technical issues at this point and the
library author is going to address all the comments before the library
would appear in Boost.
* Interface is controversial but it does not limit the library
functionality. Separating the parameters and process start is still
possible:

auto params = std::make_tuple("foo", "bar", bp::std_out > stdout,
bp::args += {"baz"});
std::make_from_tuple<bp::child>(params);

So it does not seem to be a show-stopper.

> Please notice that some of the votes were conditional, and if those
> conditions are not met, they should be counted as no votes.

Let's change all the +/- to -. Then there'll be equal count of + and -
and review manager is allowed to weight the votes:
* vote of the previous author of the library will have more weight and
the library will pass.
* even without the vote of the previous library author, removal of all
the controversial interface change requests will result in library
acceptance.

>> The most controversial part of the library is it's interface. Many
>> comments were addressing:
>> * passing arguments
>> * starting process right in `system(...)` function
>
>
> The comments were about starting the process in the child constructor.

Yes, sorry. I meant `child(...)`. And it is still solvable, see the code above.

-- 
Best regards,
Antony Polukhin