std.signal : voting has begun

[Andrei Alexandrescu]

On Monday, 6 January 2014 at 09:11:09 UTC, Dicebot wrote:
> Some time ago there have been a review for `std.signal` Phobos 
> proposal 
> (http://forum.dlang.org/thread/ujlhznaphepibgtpcoqz@forum.dlang.org#post-ujlhznaphepibgtpcoqz:40forum.dlang.org). 
> It have not received much feedback and I was a it too busy to 
> proceed with final voting at that moment but with no 
> outstanding issues to address nothing prevents that final step.
>
> Let's put 2 week deadline to refresh memories about the 
> proposal and make some decision. Voting closes at January 20th 
> 23:59 GMT 0
>
> Please take some time and help make Phobos better ;)

I am opposed to merging std.signal in its current form. It is not
at the level of documentation and implementation quality needed
for new Phobos modules.

* Documentation has numerous mistakes, omissions, disfluencies,
and colloquialisms that seriously decrease credibility of the
library. Examples:

- The first sentence (sic!) is not terminated with any
punctuation sign.

- The third sentence "They were first introduced in the Qt GUI
toolkit, alternate implementations are libsig++ or Boost.Signals2
similar concepts are implemented in other languages than C++
too." does not parse.

- Numerous symbolic identifiers (std.signal, emit, ref, etc) are
not in code font. Some should be cross-referenced, too.

- Colloqualisms should be eliminated ("very loose way", "Do some
fancy stuff", "you"/"your")

- All but one uses of "which" should be replaced with "that"

- Many, many others

* There's no synopsis at the very beginning motivating the module
with a simple example.

* There's no motivation or explanation given for RestrictedSignal
and no example of where one should use it.

* The notion of "strong" connection is not motivated and poorly
defined ("If in your application the connections made by a signal
are not that loose...")

After reading the very brief documentation I was unfortunately
left with an unclear picture of what the code offers and what are
its limitations. What is clear to me is that documentation needs
serious work that cannot probably be addressed in time for this
review cycle.

The code:

* Portions look foreign compared to the rest of Phobos, e.g.
spacing present around some operators but not others following no
apparent rule. This is quite jarring.

* Quadratic growth strategy in addSlot

* Several statements on the same line (e.g. debug (signal) { ...
}), some long enough that even github doesn't get to display
without scrolling.

* Some signed integers should be unsigned

* (Minor) redundant control flow and duplicated code in
wasConstructedFrom

* Odd whitespace in line 819

* Line 917: should that be an enforce?

* At least one more colloquialism ("Such dirty tricks...")

* Variables called "o", sigh.

Overall: I think the code is relatively easy to fix with one
careful pass, but the documentation needs a lot of work.


Andrei