-
-
Notifications
You must be signed in to change notification settings - Fork 73
PHPBCF: Show no errors were found
when there are no errors to fix.
#807
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
PHPBCF: Show no errors were found
when there are no errors to fix.
#807
Conversation
Modifies the output of PHPCBF when the exiting without fixing any errors to show a message indicating that the reasons was due to no coding standards violations in the code base.
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.
@peterwilsoncc Thanks for this PR and your willingness to contribute. I have no objections to this change.
I haven't been able to find tests for the
phpcbf
exit messages (searching for the phrase "No fixable errors were found") so I haven't added tests for this change. Please do point me to the relevant location if I have missed something.
You are correct that the Reports
part of the codebase is sorely lacking tests. While this is a blocker for larger changes to Report classes, a small change like this can still be tested manually quite easily, so I'm okay with this going in without tests covering the change.
I've manually tested the fix and all looks good.
As the phpcbf
exit codes aren't great at the moment (also see #184), I can imagine, there may be people out there who grep
the output of phpcbf
in CI to determine next steps/whether to fail a build.
While this PR would improve the ability to do so, I can also imagine this PR might possibly break such setups, so I'm going to leave the PR open for a little to allow for others to speak up if they have any concerns about this change.
If there are no objections over the next week or so, I'll merge this into 3.12.0
. If there are, we'll have to move it to the 4.0.0
milestone.
I vote for putting this into a minor release (3.x), and not waiting for the next major release (4.0). I see this as a usability improvement and not a breaking change. I don't see this text as part of the backwards compatibility layer / contract with users. The exit code is, but that's not being changed here and improvements to that are being tracked in #184. Anyone looking for strings in the output of |
Merging as there have been no objections. |
…807) Modifies the output of PHPCBF when the exiting without fixing any errors to show a message indicating that the reasons was due to no coding standards violations in the code base.
Description
Modifies the output of
phpcbf
when the exiting without fixing any errors to show a message indicating that the reasons was due to no coding standards violations in the code base.The purpose is to make it clearer to developers running
phpcbf
that the reason no errors were fixed is because there were none to fix.I haven't been able to find tests for the
phpcbf
exit messages (searching for the phrase "No fixable errors were found") so I haven't added tests for this change. Please do point me to the relevant location if I have missed something.Edit: I've pushed a change to the message based on a discussion on the ticket. As/if further input arrives, I'll make any changes as required.
Suggested changelog entry
PHPCBF exits with the message
no violations were found
when no CS violations exist in the project.Related issues/external references
Fixes #806
Types of changes
PR checklist