[CS] Add missing method scope and fix function comment style #1751
Conversation
| @@ -66,7 +66,6 @@ public static function getDateTime($value) | |||
| return $datetime; | |||
| } | |||
|
|
|||
| // @todo fix typing for $microseconds | |||
There was a problem hiding this comment.
This one was auto removed. Should we allow the @todo annotation in our CS?
There was a problem hiding this comment.
Yes, I believe this is a good idea. There will be a few of those left in while we work in 2.0. We should however remove the exception before tagging 2.0.
There was a problem hiding this comment.
I'll move these @todo annotations inside the method scope instead of keeping them as annotations :)
| @@ -12,7 +12,6 @@ | |||
|
|
|||
| class AnnotationDriverTest extends AbstractMappingDriverTest | |||
| { | |||
| // @TODO: This can be a generic test for all drivers | |||
There was a problem hiding this comment.
This one was auto removed. Should we allow the @TODO annotation in our CS?
| @@ -116,8 +116,7 @@ public function testRegisterRemovedOnNewEntityIsIgnored() | |||
| } | |||
|
|
|||
|
|
|||
| /* Operational tests */ | |||
|
|
|||
| /** Operational tests */ | |||
There was a problem hiding this comment.
Is this a false-positive forcing to be a function comment?
There was a problem hiding this comment.
This is also just a useless comment that we could remove.
| @@ -51,7 +51,7 @@ class DateCollectionType | |||
| { | |||
| use ClosureToPHP; | |||
|
|
|||
| // Note: this method is called by PersistenceBuilder | |||
| /** Note: this method is called by PersistenceBuilder */ | |||
There was a problem hiding this comment.
This should either be a proper docblock or dropped completely: the information where it's called from can change any time and can easily be inferred from the IDE. If we want to add a comment (which might be better off in the base class as it applies to all types) we should explain what the method is expected to do.
| @@ -90,7 +90,7 @@ public function convertToPHPValue($value) | |||
| return $value; | |||
| } | |||
|
|
|||
| // Note: this method is never called | |||
| /** Note: this method is never called */ | |||
| @@ -389,7 +389,7 @@ public function testQueryWhereInReferenceId() | |||
| $this->assertSame($expected, $qb->getQuery()->debug('query')); | |||
| } | |||
|
|
|||
| // search for articles that have the "pet" tag in their tags collection | |||
| /** search for articles that have the "pet" tag in their tags collection */ | |||
There was a problem hiding this comment.
TBH, the value of this comment is 0 - I can see what it's doing in the code. I suggest removing this kind of comments entirely.
| @@ -48,7 +48,7 @@ class GH1294User | |||
| /** @ODM\Field(type="string") */ | |||
| public $name = false; | |||
|
|
|||
| // Return the identifier without triggering Proxy initialization | |||
| /** Return the identifier without triggering Proxy initialization */ | |||
There was a problem hiding this comment.
The comment refers to an implementational detail of proxies, which is irrelevant in the base class. Again, I suggest dropping this comment and the ones like it.
| @@ -48,8 +48,7 @@ public function setRecId($value = null) | |||
| $this->recId = $value; | |||
| } | |||
|
|
|||
| /* Added automatically 2010-08-03 */ | |||
|
|
|||
| /** Added automatically 2010-08-03 */ | |||
| @@ -116,8 +116,7 @@ public function testRegisterRemovedOnNewEntityIsIgnored() | |||
| } | |||
|
|
|||
|
|
|||
| /* Operational tests */ | |||
|
|
|||
| /** Operational tests */ | |||
There was a problem hiding this comment.
This is also just a useless comment that we could remove.
No description provided.