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

PHPCBF: Show "No errors were found" when nothing needs to be fixed. #806

Closed
2 tasks done
peterwilsoncc opened this issue Feb 7, 2025 · 4 comments · Fixed by #807
Closed
2 tasks done

PHPCBF: Show "No errors were found" when nothing needs to be fixed. #806

peterwilsoncc opened this issue Feb 7, 2025 · 4 comments · Fixed by #807

Comments

@peterwilsoncc
Copy link
Contributor

Is your feature request related to a problem?

When running phpcbf against a clean code base, ie one following coding standards, the exit message is No fixable errors were found.

In order to confirm the code base is clear of errors, one needs to also run phpcs to determine if any issues need to be manually fixed.

As phpcbf has run the sniffs, it would be good to be informed if there are no errors found upon exit.

Describe the solution you'd like

If the code base has no errors or warnings, output No errors were found in place of No fixable errors were found.

An alternative may be: No coding standards violations were found

Additional context (optional)

@jrfnl
Copy link
Member

jrfnl commented Feb 7, 2025

Sounds loosely related to #184, as part of which the exit code is likely to change.

For the record, there are currently four situations possible:

  1. Clean code base, no errors at all.
  2. Dirty code base, all errors/warnings are auto-fixable.
  3. Dirty code base, some errors/warnings are auto-fixable.
  4. Dirty code base, no errors/warnings are auto-fixable.

These four situations are currently handled as follows:

Situation Auto-fixable Shows Exit code End result
1. Clean N/A "No fixable errors were found" 0 Clean code base
2. Dirty All Report with "Remaining 0" for each file 1 Clean code base
3. Dirty Some Report with "Remaining > 0" for some files 1 Dirty code base, non auto-fixable
4. Dirty None "No fixable errors were found" 0 Dirty code base, non auto-fixable

As part of #184, the exit code for situation 2 should change to 0.
And I'm not sure yet, but I can imagine that the exit code for situation 4, should possibly change to 1 ?

As for your proposal, if I understand it correctly, your intention is to change the message for situation 1 to "No errors were found" and to leave the other situations alone. Correct ?

@peterwilsoncc
Copy link
Contributor Author

As for your proposal, if I understand it correctly, your intention is to change the message for situation 1 to "No errors were found" and to leave the other situations alone. Correct ?

Yes, that's correct.

Thinking about it further, I am wondering if "No coding violations were found" would be clearer for developers, as it's noticeably different even when scanning. Probably bike shedding.

As part of #184, the exit code for situation 2 should change to 0.
And I'm not sure yet, but I can imagine that the exit code for situation 4, should possibly change to 1 ?

I think both of these changes make sense:

  • exit 1: the code base is dirty
  • exit 0: the code base is clean

I'm happy to make these changes is the associated PR if you give me the go ahead.

@jrfnl
Copy link
Member

jrfnl commented Feb 7, 2025

@peterwilsoncc

Thinking about it further, I am wondering if "No coding violations were found" would be clearer for developers, as it's noticeably different even when scanning. Probably bike shedding.

Maybe even No violations were found or No issues were found ? Happy to hear more opinions.

As part of #184, the exit code for situation 2 should change to 0.
And I'm not sure yet, but I can imagine that the exit code for situation 4, should possibly change to 1 ?

I think both of these changes make sense:

  • exit 1: the code base is dirty
  • exit 0: the code base is clean

Thanks for your thoughts on this. I'll keep it for consideration for issue #184.

I'm happy to make these changes is the associated PR if you give me the go ahead.

The exit code changes cannot be combined with the current change as those need to be made in a major release, i.e. in 4.0.0, while the message change can go in before that time.

@peterwilsoncc
Copy link
Contributor Author

I've pushed a change to no violations were found, if others present opinions I'll take them in to account.

@jrfnl jrfnl added this to the 3.12.0 milestone Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants