Skip to content

various #236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2016
Merged

various #236

merged 1 commit into from
Jul 1, 2016

Conversation

MBoretto
Copy link
Collaborator

Doc block, various error, fixed thanks to incredible new tools..

@noplanman
Copy link
Member

Good to merge, mostly DocBlock changes and some code changes that don't affect any behaviour.

👍

@MBoretto MBoretto merged commit 1513e97 into php-telegram-bot:develop Jul 1, 2016
@@ -33,7 +33,7 @@ class SendtochannelCommand extends AdminCommand
/**
* Conversation Object
*
* @var Longman\TelegramBot\Conversation
* @var \Longman\TelegramBot\Conversation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBoretto Did you deliberately leave all full namespaces as opposed to simplified ones?

PhpStorm automatically prefers simplified ones for me.

@akalongman @jacklul
Asking you guys too, which standard shall we go by?

@var \Longman\TelegramBot\Conversation

or

@var Conversation

(this obviously implies that we have a use Longman\TelegramBot\Conversation at the top.

@MBoretto
Copy link
Collaborator Author

MBoretto commented Jul 5, 2016

I add it because with the \ in the front and the full namespace php storm seems able to recognise the type of the variable

@noplanman
Copy link
Member

Interesting, it seems to work for me without the whole namespace if there is an appropriate use present.
So you would suggest to add the full namespace to all docblock variables?

@akalongman
Copy link
Member

Yes, I suggest add full class names including namespaces in DocBlock

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