Re: [boost] Boost summer of formal reviews

Subject: Re: [boost] Boost summer of formal reviews
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2014-03-11 13:45:02

On 11 Mar 2014 at 12:29, Borislav Stanimirov wrote:

> I am not saying that most (or any) of the libraries stuck in review 
> limbo, should pass the process, but some of them at least deserve a look 
> (again, like AFIO).

What slightly irritates me about the present review queue is that 
there is a wide disparity between the quality of the libraries in 
there, with some clearly not ready for peer review.

What I'd really, really like is if the review schedule also listed 
answers to at least the following questions so queue submitters have 
a better idea of what is demanded:

1. List of compilers supported and their earliest and latest versions 
tested upon.

2. List of operating systems supported and their earliest and latest 
versions tested upon.

3. List of architectures supported and their earliest and latest 
versions tested upon, including endian.

4. Code coverage of the test and functional suite (in percentage) 
with an auto-reject if it's not above 75%. Travis CI + coveralls.io 
will do this for you for free, no one has any excuse to not have this 
nowadays. For code not covered, break down what it consists of?

5. Link to the Continuous Integration server for the project which 
does a unit test run for every pull request or commit to the project 
(with a big red X if there is none given Travis CI is free - I'd 
auto-reject any new library without CI integration). Treble extra 
points if you also have a CI for Windows, twice that again if you 
also have a CI for Mac OS X or BSD, and multiply the result by ten if 
all unit and functional tests are 100% stable over time.

6. Do running all the unit and functional tests pass on valgrind's 
memcheck tool? Does the library pass when compiled with clang's 
static analyser?

7. Is the library C++11 aware? Do all appropriate objects have move 
constructors available?

8. Is the documentation supplied in BoostBook and using the 
*standard* Boost templates? (Hint: there are a number of libraries in 
the queue without either). Does the documentation provide at least 
the following: (i) a design rationale (ii) a tutorial (iii) examples 
of usage (iv) a reference section.

9. Is the library fully exception safe using the Abrahams guarantees? 
List all areas in which it is not, with justifications (a common one 
will be failure to always guarantee correct handling of 
std::bad_alloc).

10. Has every API been audited for worst case complexity and that 
value is listed in every API documentation page? If not, explain why 
with justifications.

11. Has every API been audited for thread safety, and the results 
listed in every API documentation page? If not, explain why with 
justifications.

12. Does the documentation include real world benchmarks for both 
compilation times and execution times demonstrating that the library 
has been to some extent tuned for performance? If not, explain why 
with justifications.

I know all these requirements are already listed in the wiki, but 
back when I was looking to peer review manage a library I found it 
hard to find answers to most of the above questions for some given 
library. Having the submitter fill them in at the point of submission 
would be hugely useful.

Just as so I eat my own dog food, here are the results to the above 
questions for my own review submission, Boost.AFIO:

1. GCC 4.6-4.8, clang 3.1-3.4, Visual Studio 2010-2013.

2. All POSIX and Windows. Tested on Linux 3.0-3.3, Windows XP - 
Windows 8, FreeBSD 10.

3. All architectures with atomic ops. Tested on ARM v7 le, 486 le to 
x64 le. Big endian ought to work, but is untested.

4. Code coverage by the CI tests is 90%. The code not covered is 
almost entirely error handling, with 60% of it being errors that 
should never happen and are only there to detect memory 
corruption/race conditions.

5. AFIO's CI is at https://ci.nedprod.com/. On each commit a full 
unit and functional test run is done for Linux, Windows, FreeBSD and 
valgrind. The full unit and functional test suite survives being soak 
tested for 24 hours with no failures.

6. Yes, AFIO is valgrind memcheck clean. AFIO has one failure in the 
clang static analyser, but this is due to bad code in 
Boost.Filesystem.

7. AFIO requires C++11, and therefore is C++11 aware. All appropriate 
objects provide move construction.

8. AFIO's documentation is 100% Boost standard, and contains all the 
specified sections.

9. The library is completely exception safe, except when multiple 
std::bad_alloc's are thrown in sequence where the process will 
probably hang due to entering an undefined state (yes, I feel ashamed 
about this, but I haven't figured out a good solution yet).

10. Every API has been audited for complexity and that value is 
listed in the documentation.

11. Every API has been audited for thread safety and that value is 
listed in 
the documentation.

12. The documentation contains real world benchmarks for execution, 
not for 
compilation time, however they are for v1.1 of AFIO and both ought to 
have significantly changed with v1.2. This will be fixed soon.

Niall

-- 
Currently unemployed and looking for work in Ireland.
Work Portfolio: http://careers.stackoverflow.com/nialldouglas/