|
|
Boost : |
From: Klemens Morgenstern (klemensdavidmorgenstern_at_[hidden])
Date: 2022-08-24 05:32:35
Boost.URL is ACCEPTED into boost without any conditions.
The review recommendations were 6 ACCEPT, 1 CONDITIONAL ACCEPT and 1 REJECT.
Many points were raised as part of the review, based on which I recommend
doing the following:
- reviewing which types/functions in the lower-level API are exposed and
which ones are not (Ruben Perez' condition)
- reviewing pct_encoded_view & encoded_path (as mentioned by Дмитрий)
- adding more verbose docs without relying on knowledge of RFC3986 on the
user side (mentioned by Richard)
- adding more simple examples (recommended by Jon Kalb)
- consider adding IRI support in the future (recommended by Rainer)
Alan & Vinnie have been very responsive and read all the reviews, which
makes it unnecessary to summarize everything.
There is only one real point of controversy, namely the only reason for
rejection written by Phil Endecott:
> More fundamentally, I'm not convinced that this allocation-avoiding
> view-rich API style is best for a general purpose vocabulary type
> like URL. URL should be a "Keep It Simple" type, accessible to
> C++ beginners, with straightforward lifetime semantics. Compare with
> std::filesystem::path, which does not try to optimise by actually
> being a view.
>
> Because of these concerns about the basic API design, I believe that
> the library should be REJECTED.
This argument has however been directly addressed in other reviews, with
Seth considering it as an appropriate choice:
> Like some of the earlier reviewers, I also have some concerns over
introducing
> non-owning vocabulary type. I do appreciate the design choice though,
because I
> think it ultimately fits the Boost ecosystem best - favoring
> performance/versatility over ease of use.
While Zach Laine considers this as the correct choice:
> Overall, I quite like the design. Lazy, lazy, lazy. The fact that
> only views into the underlying sequence are returned from the parse is
> great, and the fact that there are both owning (url::url and
> fixed-capcity owning url::static_url) and view versions of the URL
> representations is exactly what I would need as a user.
For my decision, the first factor is that more people approved of this
particular design choice than disapproved.
Secondly it seems to me that the C++ ecosystem is moving towards more
view-support.
The potential pitfalls can be addressed by coding guidelines (e.g. don't
use auto) and do not
need to be addressed by constraining libraries. Thus in finding a result, I
did not
consider Phil's argument to outweigh the other reviews which were
considering it an acceptable choice.
Other issues were the documentation, the confusion about encoded and
decoded data and IRI support.
Neither did lead to a rejection, so I'll leave it up to Alan's & Vinnie's
discretion to review & address all the
raised issues and to engage in further discussion on the mailing lists.
Thank you Dmitry, Jon, Phil, Rainer, Richard, Ruben, Seth and Zach for your
reviews
and Alan & Vinnie for providing yet another boost library.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk