2016-11-13 4:35 GMT-08:00 Bjorn Reese
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_tuplebp::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