[RFC] Add more forbiddenAnnotations #28
Conversation
lib/Doctrine/ruleset.xml
Outdated
| @package, | ||
| @since, | ||
| @subpackage, | ||
| @uses, |
There was a problem hiding this comment.
@uses is complimentary with @covers in test suites, and affects phpunit at runtime when using strict coverage checks
There was a problem hiding this comment.
Oh, I see it https://phpunit.de/manual/current/en/appendixes.annotations.html, sorry!
| name="forbiddenAnnotations" | ||
| type="array" | ||
| value=" | ||
| @api, |
There was a problem hiding this comment.
@api can be useful when dealing with BC promises or the lack thereof, but I don't think we use it for now, so it's probably ok to include it.
There was a problem hiding this comment.
If it is public and it is not marked @internal then it is already API :P
There was a problem hiding this comment.
Agreed, @api adds maintenance overhead. Any public or non-final protected is typically public API by design. At least it motivates devs to think about API & what should be exposed and what not (looking at you SF). 👍
lib/Doctrine/ruleset.xml
Outdated
| @package, | ||
| @since, | ||
| @subpackage, | ||
| @uses, |
There was a problem hiding this comment.
This shouldn't be added IMO since PHPUnit uses it for explicit coverage definition (@covers and @uses).
There was a problem hiding this comment.
Oh, I see it https://phpunit.de/manual/current/en/appendixes.annotations.html, sorry!
There was a problem hiding this comment.
👍 @uses is already whitelisted btw.
carusogabriel
force-pushed
the
forbidden-annotations
branch
from
4c9de74 to
7a34ebd
Compare
|
Ready to Merge? |
|
🚢 |
There are some annotations in https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md#7-tags that we some a few occurrences in our repos, but as minimal, I want to vote to forbidden them in our CS 😄