[text] [review] Text formal review result

From: Glen Fernandes (glen.fernandes_at_[hidden])
Date: 2020-06-26 04:10:30

Many thanks to Zach for submitting his third library, Text, for Boost
formal review. And thank you to the Boost community for participating.
You can jump to result at the end of this email.

The review period ran from June 11 to June 20 and we received a total
of four reviews (three within the period). I won't summarise
individual points, all reviews are accessible in the Boost list
archives.

1. Emil Dotchevski submitted an early review, on June 3. He voted to
accept the entire library with no conditions.
2. Rainer Deyke submitted a review on June 12. He voted to accept only
the Unicode algorithms.
3. Phil Endecott submitted a review on June 18. He voted to accept
only the UTF transcoding algorithms, and accept the Unicode layer
providing the license issue is resolved.
4. JeanHeyd Meneide submitted a review after the review period ended,
on June 21. He voted to accept the Unicode layer, and require that the
text layer (if accepted) use std::string instead.

I received no private reviews, but one email expressing concern about
the copyright issue and eligibility for review.

My thanks to Emil, Rainer, Phil, and JeanHeyd for your reviews. Thanks
also to Gavin Lambert, Vinnie Falco, Alexander Grund, Villie
Voutilainen, and everyone who participated in the discussion.

The following points had the most weight in the review result that follows:

1. No desire for the string layer

   The bar for a custom string type is high. On a minor note presents
incompatibilities with interfaces using std::string. On a major note a
string replacement should give users things they currently depend on
in std::basic_string. One of these is the ability to control dynamic
allocations. Another is working with strings larger than 2GB.

   Allocators are not only about performance; i.e. the argument that
supporting custom allocators would compromise the performance of the
implementation is not enough to justify their absence. Users have the
need to choose which area of dynamic memory their objects live in. In
some cases users have a requirement that all dynamic allocations go
through a custom (and stateful) allocator.

   JeanHeyd mentioned the now-deprecated wstring_convert functions
which, despite their shortcomings, were usable. I filed LWG 2226
(http://wg21.link/lwg2226) years ago while at Microsoft because we had
to reject its use in such a project where every dynamic allocation had
to support stateful allocators, and wstring_convert did not. The end
result was the developer reimplementing the functionality with proper
support for stateful allocators.

   Supporting customizing allocation doesn't necessarily have to
involve the current allocator model that containers or other standard
library allocator-aware facilities support. As long as it provides the
user with a similar level of control.

2. No strong acceptance for the text layer

   I won't repeat the points here. There was no strong desire
expressed to have the text layer in Boost. The compromise to have it
backed by std::string was still expressed in light of several
shortcomings.

3. Portions of the library under a non-BSL license

   Phil identified three cases where the library did not have
appropriate licensing. One was a simple omission, another has been
resolved by the relevant code being relicensed under the BSL, but the
other is still pending. We have a strict requirement for Boost
licensing of code and data in our libraries that ship in the Boost
distribution, and we cannot compromise on this. Zach understands this
(he's authored two successful Boost libraries) and I'm confident he
will address this.

4. A strong desire to have the Unicode layer in Boost

   Especially the encoding transcoding, the Unicode algorithms part of
the library was well received. The reviews - though small in number -
have convinced me of the value of accepting this layer of the library,
provided the submission in question (all code and data) is licensed
entirely under the BSL.

A minor issue:

1. For a library of this size (all three layers are of non-trivial
implementation size), I hoped for more reviews and participation.
Rather than extend the review period by an additional week, the
feedback received above lead me to believe that for Boost's and Zach's
benefit, the result below and the prescribed course of action would be
better:

The result of this review is to reject the Text library in its current
form, but to ask Zach to submit a subset of the library containing
only the Unicode layer for mini-review, incorporating the feedback
pertaining to it in this review. Where all of the code and data is
licensed only under the BSL.

The review wizard has the ability to grant a 5-day mini-review instead
of a full 10-day review, and I've petitioned Mateusz to grant that
Zach have this opportunity if he chooses for a second review of a
library with just the Unicode layer.

The text layer (backed by std::string, but ideally designed one that
does not inhibit use of custom allocators ) could be reviewed as a
separate library, to improve the chances of getting the Unicode
algorithms in the hands of Boost users.

My thanks once again to Zach for submitting his Text library for a
review, and I hope that the second time's the charm.

Glen