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

Add annotation with proper types for entity properties #394

Closed
wants to merge 3 commits into from

Conversation

@bocharsky-bw
Copy link
Contributor

commented Nov 2, 2016

The most IDEs start showing returned types on getter calls after these changes.

@bocharsky-bw

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2016

I also wonder about using @var annotation for setters and @return one for getters in Symfony Demo. What do you think about adding it?

@javiereguiluz

This comment has been minimized.

Copy link
Member

commented Nov 3, 2016

If we add the @var for properties, I guess it makes sense it add it to getters/setters too.

@@ -22,56 +22,74 @@
class Post
{
/**
* @var int

This comment has been minimized.

Copy link
@stof

stof Nov 3, 2016

Member

useless for constants. The IDE already reads the value anyway. A constant is not a variable

This comment has been minimized.

Copy link
@bocharsky-bw

bocharsky-bw Nov 3, 2016

Author Contributor

Reverted.
Thanks, Christophe!

@bocharsky-bw bocharsky-bw force-pushed the bocharsky-bw:properties-types branch from 3ebed1c to 08790e1 Nov 3, 2016
@xabbuh

This comment has been minimized.

Copy link
Member

commented Nov 4, 2016

When the properties are annotated, you shouldn't have to repeat the type for the returned values of the getter methods as IDEs are able to infer the return types then.

public function getPublishedAt()
{
return $this->publishedAt;
}
/**
* @param \DateTime $publishedAt

This comment has been minimized.

Copy link
@xabbuh

xabbuh Nov 4, 2016

Member

not needed as the type hint already provides this information

@bocharsky-bw

This comment has been minimized.

Copy link
Contributor Author

commented Nov 4, 2016

@xabbuh Do you suggest to fully revert commit about annotations for setters / getters?

@xabbuh

This comment has been minimized.

Copy link
Member

commented Nov 5, 2016

@bocharsky-bw For setters where we do not have type hints (i.e. scalar type arguments) we should keep them.

@javiereguiluz

This comment has been minimized.

Copy link
Member

commented Nov 10, 2016

@bocharsky-bw thanks for proposing this improvement. I love it when we make our code more consistent and IDE-friendly. Thanks!

@bocharsky-bw bocharsky-bw deleted the bocharsky-bw:properties-types branch Nov 10, 2016
javiereguiluz added a commit that referenced this pull request Apr 21, 2017
…ency pov" (bocharsky-bw)

This PR was merged into the master branch.

Discussion
----------

Revert "Updated the DocBlocks of the Entities from consistency pov"

This reverts #527

The reasons were described earlier in #394

Commits
-------

88d2100 Revert "Updated the DocBlocks of the Entities from consistency point of view."
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.