-
Notifications
You must be signed in to change notification settings - Fork 52
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 RequireOneLineDocComment sniff #252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm 👎 , the expected life of a @var
comment is to eventually disappear superseded by typed properties, it has almost no chances to be expanded. Method comments on the other hand might see an additional @throws
or an explanation added which would then "unwind" single-lined /** @parameter */
. This will likely lead to different styles of comments in same file and bigger diffs (currently +1/0 vs proposed +4/-1 for adding a @throws
)
Overall +1 for this: consider that `/** @var */` blocks are mostly for
things that still cannot be represented via native types, and we already
have rules for removing those.
…On Fri, May 7, 2021, 20:04 Maciej Malarz ***@***.***> wrote:
***@***.**** requested changes on this pull request.
I'm 👎 , the expected life of a @var comment is to eventually disappear
superseded by typed properties, it has almost no chances to be expanded.
Method comments on the other hand might see an additional @throws or an
explanation added which would then "unwind" single-lined /** @parameter */.
This will likely lead to different styles of comments in same file and
bigger diffs (currently +1/0 vs proposed +4/-1 for adding a @throws)
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#252 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABFVEHH6N3YFYIXDWO6LNDTMQTRPANCNFSM44JKNVEA>
.
|
@malarzm you still can't and won't be able to replace |
@ostrolucky yeah I know,but that doesn't change a thing :) |
@malarzm I'm using doctrine orm and jms-serializer in some projects. Both require their With PHP language development those annotations are slowly disappearing as it's getting replaced by native features. Having only a single one in a single doc block is becoming more and more frequent (almost everything except generics and advanced types is gone). |
Yeah but that means all (or a great majority) of comments are not inlined hence consistent across the file. IMO it'll be much harder with methods. Anyway my vote currently doesn't change a thing :) |
This should've gone to the next major: all downstream projects are broken in a patch upgrade. |
On it: #284 And I think I should write a contributing guide for this specific repository, because that's not the first time this happens. |
This reduces vertical space in code. Same as it's already done for
@var
annotations as/** @var mixed */
is now done for other annotations
/** @param mixed $param */