Skip to content

Conversation

@martinsik
Copy link
Contributor

I went through some classes and tried to add docblocks where missing. I checked the correct syntax for @param and it always has to have proper type specified. I've also seen @override which doesn't seem to be supported by phpdoc (isn't this from Java?).

This mostly applies to 2.x branch as well.

@coveralls
Copy link

coveralls commented Jan 23, 2017

Coverage Status

Coverage remained the same at 95.838% when pulling bf71b59 on martinsik:added-docblocks into 2429fb7 on ReactiveX:master.

@davidwdan
Copy link
Member

@martinsik I actually removed a lot of the doc blocks in 2.x because I think that they're unnecessary where we have clear typing. I'm also not a big fan of @inheritdoc.

@davidwdan
Copy link
Member

If anything, I would like to update the doc blocks on the operators in Observable to include more info for things like callables.

For example in the create doc block:

* @param callable(ObserverInterface $observer): DisposableInterface $subscribeAction

@martinsik
Copy link
Contributor Author

Ok, do you want to also remove @var doc blocks? I think @var and @param are actually useful.
The updated syntax for callable is still just a proposal I think phpDocumentor/fig-standards#19 (or is it already supported by IDEs?).

@davidwdan
Copy link
Member

@var doc blocks are okay. Just make a judgement call on where it's not clear from the types in the constructor.

I'm not sure if IDE support it or not, but it does make sense from a documentation perspective. The async interop is also using that notation: https://github.com/async-interop/event-loop/blob/master/src/Loop.php#L142 .

@martinsik
Copy link
Contributor Author

So lets close this PR and I'll eventually open a new one.

@martinsik martinsik closed this Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants