Boost logo

Boost :

From: René Ferdinand Rivera Morell (grafikrobot_at_[hidden])
Date: 2023-02-18 03:41:05


On Sat, Feb 4, 2023 at 9:56 PM Klemens Morgenstern via Boost
<boost_at_[hidden]> wrote:
>
> Review
> ========
>
> Please explicitly state that you either *accept* or *reject* the inclusion
> of this library into boost.
> The accept can be conditional.

The documentation is not usable as it is. There's just too much
missing to use the library past the simplest use cases. Having said
that, the implementation is good and up to par for Boost, AFAICT. My
conclusion is to CONDITIONALLY ACCEPT. The condition being to write
all the missing documentation.

> Also please indicate the time & effort spent on the evaluation and give the
> reasons for your decision.

I spent about 4 hours in total reviewing the library. I read the
documentation, playing around with the examples, and reading the code.
I made some minor changes to the build files to incorporate into my
ongoing work to adjust the Boost libraries to be fully modular with
B2. The project built without warnings, errors, or any problems at
all. So kudos for that.

> Some questions to consider:
>
> - Does this library bring real benefit to C++ developers for real world
> use-case?

Yes. Some form of templating output is ubiquitous if one needs to do
any form of input processing to generate textual output.

> - Do you have an application for this library?

Not immediately. I have some near future use cases for tooling that I
was considering something like this. Specifically in generating data
matching tooling metadata (compile commands data, package description
data, compiler response data, for ecosystem IS reference
implementations). My use cases required being able to exchange the
output format without changing the input so that I could output XML,
JSON, YAML, etc by only changing the templates (ideally).

> - Does the API match with current best practices?

What I understand of the API, yes. But that's just the "render" and using Json.

> - Is the documentation helpful and clear?

No. While I'm okay with the documentation only referencing the
mustache spec. I'm not fine with the essentially zero explanation of
what the facility does. Some questions I had while reading the
documentation:

* How would it work if I don't use Boost.Describe?
* Should I know where in the Json docs I should look for further answers?
* How do I use "render_some"?
* Why should I care about the "output_ref" API? I.e: What's for other
than it's the argument to "render_some"?

I did figure out most of the answers to the questions after reading
the test sources and implementation. But one shouldn't need to do that
to use a library.

> - Did you try to use it? What problems or surprises did you encounter?

No. Looking at the examples and tests was enough for me to understand
what was going on. And more importantly if it would fit the use cases
I'm looking to solve.

> - What is your evaluation of the implementation?

It is clean and straightforward. I learned some things about
core::string_view I didn't know (which I might use in B2 since I
copied that implementation for use there). While the comments are
minimal. They are also sufficient.

--
-- René Ferdinand Rivera Morell
-- Don't Assume Anything  -- No Supone Nada
-- Robot Dreams - http://robot-dreams.net

Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk