Re: [review][STLInterfaces] STLInterfaces review results

From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2019-12-26 19:08:50

On Thu, Dec 26, 2019 at 2:31 AM Barrett Adair via Boost <
boost_at_[hidden]> wrote:

> Many thanks are owed to all who participated in the formal review of Zach
> Laine's STLInterfaces library, especially during this busy holiday season:
>
> * Hadriel Kaplan - ACCEPT
> * Andreas Wass - ACCEPT
> * Rainer Deyke - ACCEPT
> * Arthur Gruzauskas - ACCEPT
> * Krystian Stasiowski - ACCEPT
> * Daniela Engert - CONDITIONALLY ACCEPT
> * Gavin Lambert - Technical comments
>
> First, a summary of the reviews:
>
> 1. Design
> Reviewers were unanimous in their appreciation for the design. Krystian had
> the only asterisk about the design, noting that derived class members
> should ideally be SFINAE-detected, where the library feature would SFINAE
> where the corresponding required derived feature detection fails. I have a
> different opinion about this, but we can discuss that elsewhere.
>
> 2. Implementation
> The container_interface destructor and the optional dependency on cmcstl2
> were snags for multiple reviewers. Several compile errors were discovered
> that need to be addressed.
>
> 3. Documentation
> Reviewers found the documentation to be "ok". A common request was for
> better organization of derived-class requirements, particularly with the
> container_interface template. Reviewers also lamented the lack of training
> wheels for the subject matter.
>
> 4. Tests
> Hadriel noted the lack of non-copyable value_type tests, while Daniela
> noted the lack of Boost.Build support. Hadriel was the only one who
> commented on the quality of tests ("good"), and the only one who ran them.
> Arthur did his own testing, to satisfaction. I appreciate the presence of
> SFINAE-friendliness tests and the continuous integration.
>
> 5. Usefulness
> Reviewers appreciated the usefulness of the library. Arthur is "using it
> happily", while others found it "very useful", "fairly useful", or of
> "high" usefulness. However, Krystian's response implied that this library
> may not be as useful for containers needing extreme performance.
>
> The reviews clearly demonstrate a consensus. Accordingly, I'm pleased to
> announce that STLInterfaces is CONDITIONALLY ACCEPTED as a Boost library.
>
> Conditions for acceptance:
> 1. Remove optional dependency on cmcstl2 (issue #27)
> 2. Add Boost license comments to all files, including appveyor.yml,
> .travis.yml, README.md, */CMakeLists.txt (issue #26)
> 3. Support Boost.Build (issue #25)
> 4. Replace googletest with Boost equivalent (issue #24)
> 5. Remove usage of non-existent v2_dtl members in container_interface.hpp
> (issue #22)
> 6. Include <tuple> for std::tie usage in C++14 mode (issue #21)
> 7. Fix dangling parens in container_inteface.hpp:731 (issue #20)
> 8. Make Concepts usage C++20-conformant (issue #13)
> 9. Remove the destructor in container_interface (issue #12)
> 10. Add tests for non-copyable value types (issue #30)
>
> Suggestions:
> 1. Explicitly document the signatures and return types of functions
> provided by base class templates (issue #23)
> 2. Mention the C++20 generator pattern in documentation (issue #19)
> 3. Document the library's interactions with Ranges (issue #18)
> 4. Remove unnecessary comments (issue #17)
> 5. Investigate compile error in VS2015 (msvc 19.0.3) due to issues with
> noexcept() operator in function overloading (issue #16)
> 6. Rename container_interface to sequence_container_interface (issue #15)
> 7. Change the 'contiguous' bool to an enum (issue #14)
> 8. Document the relationship between value_type operator== and
> containter_interface-defined operator== (issue #11)
> 9. Improve table/section names for better navigation of container_interface
> tutorial (issue #10)
> 10. Clean up BOOST_STL_INTERFACES_DOXYGEN usage (issue #28)
> 11. Split up fwd.hpp into multiple headers (issue #29)
> 12. Document optimization trade-offs of proxy_iterator_interface (issue
> #30)
>
> Rationale:
> 1. Condition #1 is, I feel, the appropriate default as review manager. If
> Zach feels strongly about keeping the optional dependency on cmcstl2, we
> should discuss.
> 2. Unless the library requirements webpage has fallen out of date, new
> libraries must still support Boost.Build.
> 3. A not-so-quick grep of current Boost libraries yields no `gtest`
> results. I assume that we do not allow googletest as the sole test
> framework in a library.
> 4. The other conditions should be self-explanatory.
>
> Thank you Zach for your contributions, and I look forward to working with
> you to add this library into Boost.
>
> Regards,
> Barrett Adair
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>