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

Get phpcs to run without error #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

itismadness
Copy link
Contributor

@itismadness itismadness commented Jan 26, 2024

Most of the changes here are a result of running vendor/bin/phpcbf, and then excluding all rules that still fail as they need to be manually fixed and better to do that in more PRs to keep this easy to review/merge. When reviewing, would encourage to click the option to ignore whitespace changes.

@itismadness itismadness changed the title Get phpcs to work and run without error Get phpcs to run without error Jan 26, 2024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably is a better idea not to exclude the phpcs rules and fix those separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on what you want out of phpcs until the rule violations are fixed. Do you want the output of phpcs to continue to be largely ignored until all violations are fixed, or do you want it to be listened to for the rules that are working fine atm, while slowly fixing the excluded rules over subsequent PRs. My preference would be for the former.

Copy link
Contributor

@recanman recanman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great effort!
I made a comment about excluding the phpcs rules.

Otherwise, LGTM.

@BrianHenryIE
Copy link
Contributor

This is a good idea, given the current codebase.

I think we should open an issue, something like ~"TODO: fix PHPCS exclusions".

Then hopefully we'll see PRs for each item individually.

  • Generic.Files.LineLength.TooLong
  • PEAR.Functions.ValidDefaultValue.NotAtEnd
  • PSR1.Classes.ClassDeclaration.MultipleClasses
  • PSR1.Methods.CamelCapsMethodName.NotCamelCaps
  • PSR2.Classes.PropertyDeclaration.ScopeMissing
  • PSR2.ControlStructures.SwitchDeclaration.TerminatingComment
  • PSR2.Methods.MethodDeclaration.Underscore
  • Squiz.Classes.ValidClassName.NotCamelCaps
  • Squiz.Scope.MethodScope.Missing

All those seem like things that should be fixed (sometimes I don't care much for some of the rules).

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

Successfully merging this pull request may close these issues.

3 participants