-
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
Configure "SlevomatCodingStandard.Commenting.AnnotationName" rule #311
Conversation
lib/Doctrine/ruleset.xml
Outdated
@@ -181,6 +181,8 @@ | |||
<rule ref="SlevomatCodingStandard.Classes.SuperfluousInterfaceNaming"/> | |||
<!-- Forbid suffix "Trait" for traits --> | |||
<rule ref="SlevomatCodingStandard.Classes.SuperfluousTraitNaming"/> | |||
<!-- Forbid "@inheritdoc" in favor of "@inheritDoc" annotation --> |
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.
Btw the sniff checks a lot more annotations :)
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.
Yes, of course. But it was added here just for @inheritDoc
.
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.
Yes, but the comment is not true because the sniff is not configured to solve just “inheritDoc”
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.
That's right. I guess I can improve it.
dbf3eca
to
0c4f8fd
Compare
I can not manage the test suite to have a green build. Do you know what is the issue? There seems that |
You have to update the |
I followed these docs, but as these processes were failing, I thought it was part of the same issue:
|
Yes. Here, you are trying to apply a patch on expected_report, which you touched. I think you can edit the patch to remove the part about the then run Then you press Hopefully there is an easier way but that's how I do it. @simPod , maybe you know of a simpler way? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
a270f63
to
8aaef07
Compare
I see you're making progress, and that's great, but I think you should make sure the 8.1 build passes before working on the 8.0 build, because it might involve touching |
Thank you so much for the hints. |
e0fc157
to
f15f2e4
Compare
e9958e1
to
6f5fcfd
Compare
Every time I merge something on this repo, I notice it is on the wrong branch right after merging 🤦♂️ |
Let's use the same solution as every time: I'll merge this up into 12.0.0, and then release it, that way there won't be a new 11.x Anyway, thanks for the new contribution @phansys and congrats on managing this green build 👍 |
Which branch should I have used as target? |
I documented the process here: https://www.doctrine-project.org/projects/doctrine-coding-standard/en/11.0/reference/index.html#versioning It's not quite the same as a regular library. Don't worry though, it won't be hard to fix :) |
Actually, merging up is going to be quite hard, since I introduced another rule on 12.0.x. Let me try. |
I gave up about the patch updates in my environment because setting the
There is no problem. Let me know if you prefer a new PR against 12.0.x. as now I can reproduce the steps that I've used for this PR. |
If you feel like up to doing the merge up, that would be awesome! I don't remember if you did them with Sonata, but here it's the same method:
And then you do a pull request. |
Sure. Let me try 👍 |
Here is the PR merging 11.1.x into 12.0.x: #314. |
See:
SlevomatCodingStandard.Commenting.AnnotationName
rule dbal#6015;SlevomatCodingStandard.Commenting.AnnotationName
: New sniff to check incorrect annotation names slevomat/coding-standard#1557TODO: