Added revokable trait for entities #13
Conversation
| /** | ||
| * @var bool | ||
| */ | ||
| protected $revoked; |
There was a problem hiding this comment.
protected has been preferred in order to remain consistent with League's traits implementation.
Furthermore, I don't see why inheritance of those properties would not be allowed.
There was a problem hiding this comment.
I think consistency with the upstream implementation makes sense here.
…ng fields on client entity concrete class
src/Entity/TimestampableTrait.php
Outdated
|
|
||
| /** | ||
| * | ||
| */ |
There was a problem hiding this comment.
Remove empty doc-block, please
src/Entity/TimestampableTrait.php
Outdated
| if (! $this->createdAt) { | ||
| $this->createdAt = new DateTime(); | ||
| if (method_exists($this, 'getTimezone')) { | ||
| $this->createdAt->setTimezone(new \DateTimeZone(($this->getTimezone()->getValue()))); |
There was a problem hiding this comment.
Please import DateTimeZone.
There was a problem hiding this comment.
Hm... There is a lot of parenthesis and it looks like you can remove one pair.
src/Entity/TimestampableTrait.php
Outdated
| /** | ||
| * @param DateTime $updatedAt | ||
| */ | ||
| public function setUpdatedAt(DateTime $updatedAt): void |
There was a problem hiding this comment.
Consistency fix: everywhere we are using single space before colon in return type declaration.
src/Entity/TimestampableTrait.php
Outdated
| } | ||
|
|
||
| /** | ||
| * @param DateTime $updatedAt |
There was a problem hiding this comment.
Consistency fix: we don't need that doc-block as it not gives us any more information than we have in function signature. So basically you can remove PHPDocs on all methods
src/Entity/TimestampableTrait.php
Outdated
| { | ||
| protected $createdAt; | ||
|
|
||
| protected $updatedAt; |
There was a problem hiding this comment.
Sorry, maybe I wasn't clear in my previous comment, but it's good to keep PHPDocs here (on class properties) because we don't know what is the type - in methods we know if from method signature.
|
@jguittard the same - about doc-comment we should do in other classes/traits you've added. BTW Why build is failing with latests deps? |
|
@webimpress All right, it's getting funny and annoying! :) What is it with 'doc-comment'? |
|
@jguittard these PHPDocs without any extra information: https://github.com/zendframework/zend-expressive-authentication-oauth2/pull/13/files#diff-56840550916e42595221de7dd214d849R52 If all is defined in method signature we don't need to duplicate these information in PHPDocs. |
|
Fixed. As for build failure, it has to do with https://github.com/zendframework/zend-expressive-authentication/releases/tag/1.0.0alpha3 and zendframework/zend-expressive-authentication#18 ;) |
|
It's probably failing because of #15 not being merged yet. I can't / am not allowed to merge it until the release-1.0.0 branch is created. |
|
Yes, I was currently writing so... wait & see! |
weierophinney
dismissed
Ocramius’s stale review
Explanation from author makes sense.
Added revokable trait for entities
|
Thanks, @jguittard! |
revokedbeing a field of all tables, it should be available as a property for bound entities.This PR adds a trait with
getterandsetterin every entity, allowing objects to reflect their DB entry.