On Sat, Feb 4, 2023 at 9:56 PM Klemens Morgenstern via Boost
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