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/