Skip to content
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

Fix invalid return type in docblock #1496

Closed
wants to merge 4 commits into from
Closed

Conversation

dagio
Copy link
Contributor

@dagio dagio commented Aug 26, 2015

No description provided.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3881

We use Jira to track the state of pull requests and the versions they got
included in.

@@ -183,7 +183,7 @@ public function expr()
*
* @param boolean $cacheable
*
* @return \Doctrine\ORM\AbstractQuery This query instance.
* @return QueryBuilder
Copy link
Contributor

Choose a reason for hiding this comment

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

@DHager
Copy link
Contributor

DHager commented Aug 26, 2015

For "fluent" chaining, can you use @return self or @return $this , or are those only recognized by certain IDEs?

@dagio
Copy link
Contributor Author

dagio commented Aug 26, 2015

@DHager Yes. For the moment I just fixed the one that were incorrect so know all method returning $this have QueryBuilder as return type so in IDEs we have no more issues reported. @returns $this and @returns self are not used a lot in the project so I would let you guys decide the right choice.

@dagio
Copy link
Contributor Author

dagio commented Aug 27, 2015

@Koc @DHager I updated in favor of @return $this as recommended in PSR-5 draft

@Koc
Copy link
Contributor

Koc commented Aug 27, 2015

@dag-io can you check consistency in other classes, like https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/AbstractQuery.php#L343 and others?

@dagio
Copy link
Contributor Author

dagio commented Sep 6, 2015

@Koc, it's all good now

@TomasVotruba
Copy link
Contributor

👍

@@ -183,7 +183,7 @@ public function __construct(EntityManagerInterface $em)
*
* @param boolean $cacheable
*
* @return static This query instance.
* @return $this This query instance.
Copy link
Member

Choose a reason for hiding this comment

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

static was correct here

Ocramius added a commit that referenced this pull request Dec 11, 2015
@Ocramius Ocramius closed this in 624fcb4 Dec 11, 2015
@Ocramius
Copy link
Member

I manually applied the patch after a rebase and reverting $this to self or static (depending on context).

Thanks @dag-io, sorry for the long delay!

@Ocramius Ocramius self-assigned this Dec 11, 2015
@Ocramius Ocramius added this to the 2.6.0 milestone Dec 11, 2015
@TomasVotruba
Copy link
Contributor

Thank you @Ocramius

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants