Hi, everyone. This is a summary of issues we have opened during the review so anyone interested can track them. * Public grammar namespace: https://github.com/CPPAlliance/url/issues/444 I find the creation of a mini-parsing library and the expectation that
users will use this mini-library to write their own parsers to be somewhat problematic.
Regarding the parser interface, I feel this should mature into its own
library.
I'm not very keen on exposing the parsing infrastructure and helper
functions
I can see other public types in the reference such as recycled,
recycled_ptr and aligned_storage, which again have the feeling should not be exposed as public types.
* parse_url vs. parse_uri:
https://github.com/CPPAlliance/url/issues/443
When I call parse_uri(), I get a result of type result
isn't the function spelled parse_url()
In contrast to others, I think the choice of Nomenclature, namely URL
everywhere except in the grammar domain (the parse function names) is a very elegant one
* Documentation issues: https://github.com/CPPAlliance/url/issues/442 (About 20 minor issues with the docs) * Exploring IRIs: https://github.com/CPPAlliance/url/issues/441 The lack of IRI support is unfortunate. It's 2022; we should all
be writing software with Unicode support by default. However, this can be built on top of Boost.URL, and isn't needed in all cases.
* Add lowercase/uppercase in pct_encode_opts: https://github.com/CPPAlliance/url/issues/440 In the past I have had situations where it was necessary to specify the
case for hex letters
* Optional query values: https://github.com/CPPAlliance/url/issues/439 I would be inclined to use a std::optional for the value instead. * Name of the decoding view is confusing: https://github.com/CPPAlliance/url/issues/438 I would expect a "decoded_view" to refer to underlying encoded data pct_encoded_view is an unfortunate choice for a view that actively decodes
URL-encoded characters
I did also find the naming situation for encoded/decoded a bit confusing. I
would expect the opposite.
* Using std::string in url: https://github.com/CPPAlliance/url/issues/437 In your non-view url object, you seem to allocate storage using new char[].
Why not use a std::string?
* url::persist docs: https://github.com/CPPAlliance/url/issues/436 so url_view doesn't own the underlying storage, except when it
does as a result of this mysterious method. What's the rationale for this?
* url lifetimes: https://github.com/CPPAlliance/url/issues/435 There's no indication here that r is a non-owning view and that
the caller is responsible for keeping s alive for the lifetime of r.
* Examples: https://github.com/CPPAlliance/url/issues/419 I miss an example (and possibly a page) about composing and mutating URLs I'd add a page and an example on percent encoding functions small examples may be even more useful to newcomers. I found resolve a bit mystifying. (...) it will be a big help to have
inspirational examples
Perhaps the examples would be better when fleshed out in long form * pct-encoding overloads: https://github.com/CPPAlliance/url/issues/417 I'd consider exposing higher-level functions here, maybe taking a lvalue
reference to a std::string
* container names: https://github.com/CPPAlliance/url/issues/416 I've found the naming for view containers confusing (e.g. params and
params_view).
I would suggest leaving url and url_view as-is, and replacing the "view"
part for "const" for the rest of the containers
Em sex., 12 de ago. de 2022 às 23:30, Klemens Morgenstern via Boost < boost@lists.boost.org> escreveu:
Hi all,
the formal boost review of the boost.url library starts today, the 13th of August and will last until and including the 22nd of this month.
The library has been developed by Vinnie Falco and Alan de Freitas. The master branch is frozen during the review and can be found here: https://github.com/CPPAlliance/url
The current documentation can be found here: https://master.url.cpp.al/
Boost.URL is a portable C++ library which provides containers and algorithms which model a "URL" and understands the various grammars related to URLs and provides for validating and parsing of strings, manipulation of URL strings, and algorithms operating on URLs such as normalization and resolution.
Please explicitly state that you either *accept* or *reject* the inclusion of this library into boost. Also please indicate the time & effort spent on the evaluation and give the reasons for your decision.
Some questions to consider:
- Does this library bring real benefit to C++ developers for real world use-case? - Do you have an application for this library? - Does the API match with current best practices? - Is the documentation helpful and clear? - Did you try to use it? What problems or surprises did you encounter? - What is your evaluation of the implementation?
More information about the Boost Formal Review Process can be found at: http://www.boost.org/community/reviews.html
The review is open to anyone who is prepared to put in the work of evaluating and reviewing the library. Prior experience in contributing to Boost reviews is not a requirement.
Reviews can also be submitted privately to me, and I will not disclose who sent them. I might however cite passages from those reviews in my conclusion.
Thanks to Vinnie & Alan for providing us with a new library & thanks for all the reviews in advance.
Klemens
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Alan Freitas https://github.com/alandefreitas