Re: [boost] [CallableTraits] Review results

Subject: Re: [boost] [CallableTraits] Review results
From: Barrett Adair (barrettellisadair_at_[hidden])
Date: 2017-07-07 01:39:37

On Sun, Apr 16, 2017 at 11:36 PM, Louis Dionne via Boost
<boost_at_[hidden]> wrote:
> Dear Boost,
>
> I am pleased to announce that the CallableTraits library is CONDITIONALLY
> ACCEPTED into Boost. I counted the following formal reviews:
>
> - Bruno Dutra: Yes
> - Emil Dotchevski: Yes
> - John P. Fletcher: Yes
> - Klemens Morgenstern: Yes
> - Peter Dimov: Yes
> - Zach Laine: Yes
>
> The following people also participated to the review by providing informal
> feedback:
>
> - Edward Diener
> - Gavin Lambert
> - Niall Douglas
> - Paul Fultz II
> - Tim Song
>
> I also based my decision on previous experience I've had using the library,
> and some issues we faced in the Standard Committee where we could have used
> CallableTraits. I'd like to thank everyone who participated to the review
> for their feedback, and also Barrett Adair for his submission.
>
> The following lists the conditions for acceptance, and also summarizes the
> general feedback received during the review. GitHub issues are leveraged
> to make the tracking of these comments easier. Below, whenever I refer to
> #XYZ, the corresponding ticket can be found at
>     https://github.com/badair/callable_traits/issues/XYZ
>
> To see all the issues that were discussed during the review, you can search
> for tickets labeled with `review` on GitHub (please allow some time for
> Barrett to update the new issues I just filed):
>     https://github.com/badair/callable_traits/issues?page=1&q=+label%3Areview
>
> Conditions for acceptance
> -------------------------
> The following are points that must be addressed for the library to be
> accepted. Once all of the associated GitHub issues are closed, I ask
> that the author post to this list with links to the closed issues.
> Unless major problems (having to do with the closed issues) are raised
> at that time, the library will be automatically accepted into Boost,
> no additional review required.

Dear Boost,

I have implemented the conditions for acceptance in CallableTraits. I
have also addressed several, though not all, of the suggestions raised
during the review. The changes are present on the GitHub master
branch. Please confirm that the conditions have been met. I have
detailed the changes below, with GitHub issues linked.

>
> - The library needs to clearly document the advantages of CallableTraits
>   over FunctionTypes. Concrete examples must be given (at least one,
>   preferably two). See #117.
>

This condition has been met. Advantages over FunctionTypes are now
listed more clearly. One concrete example is now present inline in
docs. Another concrete example is linked (Eraserface), which is a
re-implementation of an extended Boost.FunctionTypes example.
https://github.com/badair/callable_traits/issues/117

> - The level of C++ conformance required for the whole library needs to be
>   clearly stated in the documentation. Individual components should only
>   document divergences from that base level of conformance. The current
>   state of things is quite confusing for users. See #118.
>

This condition has been met. Additionally, an extensive compatibility
matrix is now present in the docs.
https://github.com/badair/callable_traits/issues/118

> - The "algorithmic" parts of the library must be removed, since they are
>   out of scope. This has already been partly done in issue #114, but the
>   following algorithms must be discussed too (let's follow up in #114):
>     + args_at
>     + clear_args
>     + pop_front_args
>     + push_front_args
>     + expand_args_right
>     + expand_args_left
>

This condition has been met.
https://github.com/badair/callable_traits/issues/114

> - The lazy metafunctions and predicate traits should be SFINAE-friendly,
>   just like the aliases are. I think this inconsistency is a big deal,
>   enough to be a condition for acceptance. See #134.
>

This condition has been met, with excellent test coverage.
https://github.com/badair/callable_traits/issues/134

> Summary of the feedback received
> --------------------------------
> The following is a compilation of other points that were made during the
> review, but which are not requirements for acceptance. I trust, but do not
> require, that all of those points will be considered by the author:
>
> - There is a loss of information with the `args` metafunction. For example,
>   `void(T)` and `void(T, ...)` both give the same result. See #132.

This suggestion has not yet been addressed.

> - Consider adding a way of obtaining the parameter list of a function
>   without the type of `*this`. See #133.

This suggestion has not yet been addressed.

> - The documentation uses the terms of art "implementation defined" and
>   "undefined behavior" incorrectly. See #103 and #131.

This suggestion has been implemented.
https://github.com/badair/callable_traits/issues/103
https://github.com/badair/callable_traits/issues/131

> - The non-reference documentation should present at least one real-world
>   use case. See #116.

This suggestion has not yet been addressed.

> - The library should handle top-level cv-qualifiers. See #106.

This suggestion has not yet been addressed.

> - Consider supporting different calling conventions (e.g. `__cdecl`).
>   See #102.

This suggestion has not yet been addressed.

> - Consider adding a pre-C++17 implementation of `is_invocable` and friends.
>   See #119.

This suggestion has been implemented.
https://github.com/badair/callable_traits/issues/117

> - The documentation should clearly state that the library can be used
>   without the rest of Boost. See #99.

This suggestion has been addressed, although perhaps not fully.
https://github.com/badair/callable_traits/issues/99

> - Consider combining `expand_args` and `args` into a single metafunction
>   with a default "output" parameter of `std::tuple`. See #126.

This suggestion has been implemented.
https://github.com/badair/callable_traits/issues/126

> - Consider adding `make_fn` and `make_mem_fn` metafunctions to construct
>   a signature from a return type and a tuple of the arguments. See #113.

This suggestion has not yet been addressed.

> - Consider renaming some traits, like `is_reference_member`. See #108.

This suggestion is valid and appreciated, but will not be implemented.

> - Reword some parts of the documentation that look odd due to the use of
>   passive tense. See #105.

This suggestion has been implemented.
https://github.com/badair/callable_traits/issues/105

> - Consider adding a version of `apply_member_pointer` which carries
>   cv-qualifiers. See #138.

This suggestion has not yet been addressed.

> - Consider adding `remove_member_cv_ref` and `copy_member_cv_ref`.
>   See #139.

This suggestion has not yet been addressed.

> - Why does `qualified_parent_class_of` always returns a const& for PMDs?
>   See #112.

This suggestion has not yet been addressed.

> - Why does `qualified_parent_class_of` return a ref-qualified type even
>   for unqualified PMFs? See #112.

This suggestion has not yet been addressed.

> A few of my own comments:
> - Consider using alphabetical order in the table of contents of the
>   reference. See #135.

This suggestion has been implemented.
https://github.com/badair/callable_traits/issues/135

> - Consider renaming `apply_member_pointer` to something like
>   `make_member`. See #136.

This suggestion has not yet been addressed.

> - Consider dropping the `parent_` in `parent_class_of`, or adding a
>   note to justify the name. See #137.

This suggestion has been implemented.
https://github.com/badair/callable_traits/issues/137

> Again, thanks to everyone who participated in the review, and
> congratulations to Barrett for his work!
>
> Regards,
> Louis Dionne
>
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost