Quantcast

[CallableTraits] Review results

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[CallableTraits] Review results

Boost - Dev mailing list
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.

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

- 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

- 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.

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.
- Consider adding a way of obtaining the parameter list of a function
  without the type of `*this`. See #133.
- The documentation uses the terms of art "implementation defined" and
  "undefined behavior" incorrectly. See #103 and #131.
- The non-reference documentation should present at least one real-world
  use case. See #116.
- The library should handle top-level cv-qualifiers. See #106.
- Consider supporting different calling conventions (e.g. `__cdecl`).
  See #102.
- Consider adding a pre-C++17 implementation of `is_invocable` and friends.
  See #119.
- The documentation should clearly state that the library can be used
  without the rest of Boost. See #99.
- Consider combining `expand_args` and `args` into a single metafunction
  with a default "output" parameter of `std::tuple`. See #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.
- Consider renaming some traits, like `is_reference_member`. See #108.
- Reword some parts of the documentation that look odd due to the use of
  passive tense. See #105.
- Consider adding a version of `apply_member_pointer` which carries
  cv-qualifiers. See #138.
- Consider adding `remove_member_cv_ref` and `copy_member_cv_ref`.
  See #139.
- Why does `qualified_parent_class_of` always returns a const& for PMDs?
  See #112.
- Why does `qualified_parent_class_of` return a ref-qualified type even
  for unqualified PMFs? See #112.

A few of my own comments:
- Consider using alphabetical order in the table of contents of the
  reference. See #135.
- Consider renaming `apply_member_pointer` to something like
  `make_member`. See #136.
- Consider dropping the `parent_` in `parent_class_of`, or adding a
  note to justify the name. See #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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [CallableTraits] Review results

Boost - Dev mailing list
Dear Boost,

Thanks again to everyone who participated in the CallableTraits
review. I am continually amazed not only by the depth of knowledge and
experience present on this mailing list, but also by your willingness
to share it with others in your spare time. I've learned so much from
the code, publications, presentations, and discussions from so many
members of this community. I am so grateful that the work of this
community is freely available to all.

Thanks especially to Louis, for the effort, patience, and goodwill you
put forth in this review process, and also for encouraging me to get
involved with the Boost project over a year ago. Your gracious
tolerance of my interest in your own project has taught me about more
than just metaprogramming.

I will continue working on CallableTraits until the conditions for
acceptance are fulfilled, and each of the additional points are
addressed as well.

Taking part in this tradition has been one of the highest honors of my
life. I look forward to reuniting with many of you in Aspen next
month.

Sincerely,
Barrett Adair

On Sun, Apr 16, 2017 at 11:36 PM, Louis Dionne via Boost
<[hidden email]> 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.
>
> - The library needs to clearly document the advantages of CallableTraits
>   over FunctionTypes. Concrete examples must be given (at least one,
>   preferably two). See #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.
>
> - 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
>
> - 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.
>
> 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.
> - Consider adding a way of obtaining the parameter list of a function
>   without the type of `*this`. See #133.
> - The documentation uses the terms of art "implementation defined" and
>   "undefined behavior" incorrectly. See #103 and #131.
> - The non-reference documentation should present at least one real-world
>   use case. See #116.
> - The library should handle top-level cv-qualifiers. See #106.
> - Consider supporting different calling conventions (e.g. `__cdecl`).
>   See #102.
> - Consider adding a pre-C++17 implementation of `is_invocable` and friends.
>   See #119.
> - The documentation should clearly state that the library can be used
>   without the rest of Boost. See #99.
> - Consider combining `expand_args` and `args` into a single metafunction
>   with a default "output" parameter of `std::tuple`. See #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.
> - Consider renaming some traits, like `is_reference_member`. See #108.
> - Reword some parts of the documentation that look odd due to the use of
>   passive tense. See #105.
> - Consider adding a version of `apply_member_pointer` which carries
>   cv-qualifiers. See #138.
> - Consider adding `remove_member_cv_ref` and `copy_member_cv_ref`.
>   See #139.
> - Why does `qualified_parent_class_of` always returns a const& for PMDs?
>   See #112.
> - Why does `qualified_parent_class_of` return a ref-qualified type even
>   for unqualified PMFs? See #112.
>
> A few of my own comments:
> - Consider using alphabetical order in the table of contents of the
>   reference. See #135.
> - Consider renaming `apply_member_pointer` to something like
>   `make_member`. See #136.
> - Consider dropping the `parent_` in `parent_class_of`, or adding a
>   note to justify the name. See #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

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Loading...