async-interop / event-loop

An event loop interface for interoperability in PHP.
MIT License
169 stars 9 forks source link

Improve enable() doc and mention it also in the class level phpdoc block #150

Closed kelunik closed 7 years ago

kelunik commented 7 years ago

Thanks @nrk for mentioning it again.

nrk commented 7 years ago

Correct me if I'm wrong, but the behaviour of onReadable(), onWritable() and such methods still seems kind of vague to me. I think it should be stated clearly in their docs if the watcher being returned is already enabled by the driver or the underlying implementation (e.g. no need for the user to call enable() after onReadable()) or not.

kelunik commented 7 years ago

I can add a note to everything that creates a watcher, too.

WyriHaximus commented 7 years ago

The current changes make the default behavior a lot clearer but also adding a note to everything that creates a watcher is a good idea :+1:

nrk commented 7 years ago

@kelunik IMHO simply specifying that "Watchers created by this method MUST be immediatly marked as enabled." (or something along these lines) in the phpdocs of onWritable(), onReadable(), onSignal(), defer(), delay(), repeat() is more than enough.

This is actually different from the behaviour described for enable(), which tells how the driver should act when enabling a watcher.

kelunik commented 7 years ago

@nrk I've added a note to every method creating a watcher and have clarified enable and disable.

kelunik commented 7 years ago

Ready to merge?