** signals2 review results ** I am pleased to announce that the signals2 library by Frank Mori Hess is accepted into boost, pending some conditions outlined below are met. I would like to thank the author for his work on this library, and for his very responsive participation in the review. I would also like to thank Franz Alt, Jean-Cristophe Benoist, Vicente Botet, Fabio Fracassi, Johan Råde, Ravikiran Rajagopal, Andrew Webber, and Jesse Williamson for submitting reviews, and Terry Golubiewski, Michael Marcin, and Hansjörg for additional comments regarding the library. There were 5 votes cast for the acceptance of this library, and 3 against accepting the library at this time. The reviewers that were against immediate inclusion of signals2 suggested that the library should be accepted when several stated problems are addressed adequately. Overall, the library was regarded as a much needed thread-safe successor of Boost.Signals. Those that have tried switching from Boost.Signals to signals2 have found the switch rather painless (at least in the cases that don't involve the Boost.Signals trackable interface), and some reported using signals2 and/or its previous versions in production code. Some additional improvements that were mentioned are an automatic connection management system based on shared_ptr, and a change of namespace (which avoids problems with QT). Additionally, many requests were made for improvements in the documentation, as well as some improvements in thread-related testing and some possible changes to the interface. Frank has addressed many of the issues in the review, and in many cases offered concrete steps to address the problems. Following is an overview of the most prominent issues that were raised. These issues must be addressed before this library is included in the trunk and subsequent releases: ** Boost.Signals -> signals2 transition ** There needs to be a concrete plan of migration from the current version of Boost.Signals to signals2. The almost-decided-on route is to have both Boost.Signals and signals2 co-exist in Boost for a few releases, with Boost.Signals getting deprecated. From reading the reviews, it looks like no one has a problem with this, so it seems like it's just a matter of an agreement between the library maintainers that gets posted in the documentation / on the lists. The bigger part of the problem is to make sure that users of Boost.Signals can transition to signals2 as easily as possible. Given the reports so far, this process is indeed easy, with the biggest exception being the removal of the trackable interface in signals2 (which was removed because it is not thread-safe). Frank has indicated that he will put it back into the library and make it available for single-threaded signals as a way to ease porting for Boost.Signals users, which sounds like a great step. My biggest concern here is that the signals2 interface is "gotten right" (with respect to compatibility with Boost.Signals) by the time it is included in Boost, so that porting is both easy and a fixed target. Given how easy people have found porting their code so far, it looks like restoring the trackable interface might is all that is needed. I would discourage any further breaking changes to the interface (one thing mentioned was dropping some of the template parameters), but I also recognize the opportunity to improve the interface in response to the review. Any signals2 interface changes that significantly break compatibility with Boost.Signals should be considered very carefully (even if Boost.Signals will stay on for a while), and if possible, counterbalanced by a compatibility layer put in to ease the transition for Boost.Signals users. ** Documentation ** Pretty much all reviewers noted the documentation as lacking with regard to thread-safety aspects of signals2 and other added functionality. Frank has clarified many of the questions that came up during the review, and outlined a plan of integrating the requested information into the documentation. Specific items are outlined in my notes at the end of this mail. ** Tests ** Some reviewers also noted a lack of tests for thread-related functionality. I agree that whatever can be reasonably tested should be tested. Frank has suggested he will try to adapt some of the Boost.Thread tests where adequate. As stated, the above issues should be addressed before moving the signals2 library to the boost trunk. I don't anticipate significant problems because: * the trackable interface has already been brought into signals2 at one point in the past * all documentation related questions have been clarified during the review, and Frank's suggestions for updating the documentation were agreeable * Boost.Thread tests should provide some insight into how signals2 thread-safety could be more thoroughly tested. Also, several reports have been made of signals2 already being successfully used in production settings, which speaks to its reliability. I strongly suggest that Frank make an announcement when the changes are completed, so that those interested can make sure that their concerns have been adequately addressed. Given Frank's record as a very responsive maintainer of this library in the past, and his commitment towards the peer-review process (he insisted on having this library reviewed even though it could have technically been included with Doug Gregor's approval as an upgrade to Boost.Signals), and considering that one of the reviewers has already offered to review changes to the library, I anticipate that a de facto mini-review of the library will happen at that point even though I am not requiring one formally. Finally, below are some notes on suggested changes I extracted while reading through all of the reviews. Square brackets contain initials of some of the reviewers that mentioned the issue, and double curly braces my personal comments. This is by no means a record of everything that has been mentioned, but perhaps the author will find it useful while making the post-review changes. To be clear, the below changes are not required for the inclusion of signals2 into the boost trunk, except as they pertain to the required changes outlined above. -- Implementation -- ---- * source code could be formatted more clearly [FA] * address efficiency problems where signals2 is significantly slower than signals [FA] {{ while efficiency is important, and ideally signals2 efficiency for single-threaded use cases converges to that of Boost.Signals, this is one area where improvements can painlessly continue to happen after the integration of signals2 into the trunk (as long as they don't require interface changes), so I don't consider this a priority at this point }} * clean macro names used with Boost.Preprocessor [VB] -- Interface -- ---- * portable syntax - either establish that the syntax works (and on what compilers) or consider deprecating [JW] * use Boost.Parameter for signal template parameters due to so the user doesn't have to specify default template parameters. [VB][FA] {{ if there are any arguments against changing the signal class template to use Boost.Parameter directly, you could still provide a metafunction that uses Boost.Parameter to provide the signal type. This would allow, e.g.: signal_type<void(int, int), mutex_type<signals2::mutex> >::type my_signal; }} * use template parameters such as single_threaded or multi_threaded instead of the mutex type. [vb] {{ it should be possible to allow the Mutex parameter to be either a mutex class or a special (or policy) type that maps to a particular mutex }} {{ this suggestion was given in conjunction to a proposal for allowing external locking mechanisms, but it seems like external locking should be possible with what the library already offers - perhaps an example of this would be useful }} -- Documentation -- * update code examples in tutorial to contain fixes already present in the package [JW] {{ if you aren't already, consider using the quickbook import facility to keep the compiled code snippets and docs in sync }} * FAQ should be mentioned earlier / information in the FAQ should be more visible [JW][VB] * while the Beginner/Intermediate/Advanced designations have been found useful [FA], the tutorial has also been found readable beginning to end without much use for the designations [JW] * clarify call ordering between grouped / ungrouped slots [JW] * clarify optional_last_value [JW] [AW] * clarify the way slot::track works [VB][JW] {{ the documentation says we "pass the shared_ptr to the slot via its slot::track method before connecting", but the clarification given to Jesse Williamson also explains that the *pointer* held by shared_ptr is passed to the slot_type constructor. This seems easy to trip over - some syntactic sugar that avoids using the shared_ptr twice in the expression in two different ways might be helpful }} * offer / point to some documentation on, and provide an example for postconstructible / predestructable / deconstruct_ptr. [FA][AW][VB][FF] * document / provide example for extended_slot_type [FA] [FF] * expand the documentation of various mutexes that can be used and their properties [VB] also in regards with co-use with Boost.Thread [FF] {{ I can find the mutexes explained in the reference documentation, but an overview at the tutorial level would be nice }} * expand discussion on thread-safety: ** how exactly is signals2 thread-safe? [JB][RR] ** explain behavior in various threading circumstances [JB] * mention the benefits of the new connection management based on shared_ptr over old trackable interface (e.g., allowing non-intrusive use with 3rd party classes) [FF] * consider making the portable syntax examples less prominent [FF] -- Testing -- * add multi-threading tests [JB] * include performance tests [VB] Again, thanks and congratulations to Frank Mori Hess, and thanks to all of the reviewers. Kind regards, Stjepan