Auto-add fullstop to @throws if rule is used#3455
Auto-add fullstop to @throws if rule is used#3455pascalheidmann wants to merge 3 commits intosquizlabs:masterfrom
Conversation
jrfnl
left a comment
There was a problem hiding this comment.
I have no opinion on the merit of this PR, just left some comments to get it into a mergable state in case the PR would be considered as viable.
| case 'ExecutableFileUnitTest.2.inc': | ||
| case 'ExecutableFileUnitTest.4.inc': | ||
| return [1 => 1]; | ||
| return [1 => 0]; |
There was a problem hiding this comment.
Not sure what you're trying to do here, but this change should be undone as this is the test file for an unrelated sniff.
There was a problem hiding this comment.
I'm not really sure what changed either but without this change this unit test fails...
There was a problem hiding this comment.
How did you run the tests to come to that conclusion ?
The last CI build passed without problems, so unless you changed something either in the test files or in the sniff, there should be no reason why the tests would fail.
There was a problem hiding this comment.
FYI - this particular test is skipped in a few cases, could it be that the way you are running the tests falls under the "skip" conditions, but isn't identified correctly ? In that case, fixing the condition would be a good thing, but that should still be done in a separate PR as it would constitute a different decision point than the decision for whether or not to accept this PR.
|
@jrfnl comments are appreciated :) edit: looks like the test is broken for some time and unrelated to this PR. |
|
Thanks for the PR, but this is not the kind of auto-fix I like PHPCS to do as I can't be sure a full stop is required here. Changing comments is something I think is best left to manual code review. |
Automatic fix for cases like
will be