Subject: Re: [boost] [Boost-announce] [metaparse] Review period starts May 25th and ends June 7th
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-05-31 08:01:36
On 19 May 2015 at 23:39, Christophe Henry wrote:
> - What is your evaluation of the design?
This isn't my field, so I can't say. It looks clever and useful to
those needing such things (I don't).
> - What is your evaluation of the implementation?
* Doesn't follow Boost naming conventions (macro names, namespace,
* Doesn't appear to actually be a standalone library, but some
* Doesn't use Boost.Build.
* Doesn't make use of Appveyor for MSVC per commit testing, if it
supports MSVC (I couldn't tell).
* I got very confused trying to find his unit testing. The link
provided was something like
no unit testing was obvious. I did a github search, and unit testing
Now I am confused, because if I go to:
... that does indeed look a bit like a Boost directory layout but
missing include and src. Which suggests we were given the wrong link,
or Metaparse is split across two very similar looking paths, or
something else weird is going on.
So, please please just use the Boost directory layout, and make
Metaparse a standalone library without all the other stuff.
Let me put this more clearly: If it's not up for peer review, it
shouldn't be in the git repo.
* I don't get why he's using Boost.Test, and then uses
BOOST_MPL_ASSERT all over the place? The need for Boost.Test seems
In terms of really standout good things:
* I very much like that he has version namespacing. All Boost
libraries should do this by default.
> - What is your evaluation of the documentation?
* Not in BoostBook.
* Missing Design Rationale.
* Academic publications shouldn't be on the front page of the docs
(maybe an Appendix?). I also dislike the frequency the docs say "go
read this paper for the thing you actually want to know". Just tell
me what I want to know, don't make me go search a linked paper.
* I have absolutely no idea what compilers, or their versions, this
works on. I saw a mention of GCC 4.6 working. That's it.
* The benchmarking at
show me more than GCC 4.6, nor a graph showing how performance scales
to complexity. I also want to know performance with optimisation
enabled, and with precompiled headers enabled. Some mention of
how much my binaries might bloat would be useful. Extra brownie
points for some compiler memory consumption figures, as this sort of
metaprogramming at scale can cause problems for 32 bit systems.
In terms of really standout good things:
* The tutorial is really great. Lots of baby steps with plenty of
explanation each worked example. I only wish I had the time to do
this for AFIO.
> - What is your evaluation of the potential usefulness of the library?
Not my domain of expertise.
> - Did you try to use the library?
> - With what compiler?
> - Did you have any problems?
> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
About an hour reading the docs and the source code.
> - Are you knowledgeable about the problem domain?
No, definitely not.
My personal recommendation is that you withdraw this library from
review immediately, and go turn it first into an actual standalone
Boost library complying with Boost naming guidelines in Boost git
modular format. Then come back to us. Otherwise you're risking the
rejection due to non-quality problems which would be a shame.
If you proceed with this review with the library in its current
state, then I recommend rejection as this is not a Boost library in
its present organisation. Which is a real shame, as the code looks to
be of high quality, the quick start, user manual and especially
tutorial are fantastic, and if properly named and laid out and not
some internal implementation library of a superlibrary, this could be
a valuable addition to Boost.
-- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk