Skip to content
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

fix: broken sniff `EmptyCommentSniff` #249

Merged
merged 3 commits into from Aug 18, 2019

Conversation

@nunomaduro
Copy link
Owner

commented Aug 16, 2019

Q A
Bug fix? yes
New feature? no
Fixed tickets #62 #59 and others...

This sniff EmptyCommentSniff doesn't work well in legacy projects. Tried to contact the owner of the project here slevomat/coding-standard#688, but the owner doesn't have to have this sniff fixed.

The only solution, for now, is remove the sniff. What do you guys think?

@nunomaduro nunomaduro added the bug label Aug 16, 2019

@nunomaduro nunomaduro requested review from Jibbarth and olivernybroe Aug 16, 2019

@olivernybroe

This comment has been minimized.

Copy link
Collaborator

commented Aug 16, 2019

I agree that this error is not something that is acceptable.
Another solution would be to fix the error in a edition of the sniff locally?

This could maybe also be "fixed" by using the approach in this issue #242?

@nunomaduro

This comment has been minimized.

Copy link
Owner Author

commented Aug 17, 2019

@olivernybroe Can you double check this?

@nunomaduro nunomaduro merged commit 26878e8 into master Aug 18, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@nunomaduro nunomaduro deleted the fix/fatal-error-on-unclosed-comments branch Aug 18, 2019

@nunomaduro nunomaduro changed the title fix: removes broken sniff `EmptyCommentSniff` fix: broken sniff `EmptyCommentSniff` Aug 18, 2019

@olivernybroe

This comment has been minimized.

Copy link
Collaborator

commented Aug 19, 2019

@nunomaduro sorry didn't have time to check it :)
The code looks great, but haven't tried it locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.