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

PHPBCF: Show no errors were found when there are no errors to fix. #807

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

peterwilsoncc
Copy link

@peterwilsoncc peterwilsoncc commented Feb 7, 2025

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 [INSERT FINAL MESSAGE HERE] when no CS violations exist in the project.

Related issues/external references

Fixes #806

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.

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.
@jrfnl jrfnl added this to the 3.12.0 milestone Feb 7, 2025
Copy link
Member

@jrfnl jrfnl left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHPCBF: Show "No errors were found" when nothing needs to be fixed.
2 participants