Skip to content

Get phpcs to run without error - #153

Open
itismadness wants to merge 1 commit into
monero-integrations:masterfrom
OPSnet:chore-phpcs
Open

Get phpcs to run without error#153
itismadness wants to merge 1 commit into
monero-integrations:masterfrom
OPSnet:chore-phpcs

Conversation

@itismadness

@itismadness itismadness commented Jan 26, 2024

Copy link
Copy Markdown
Contributor

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
Comment thread phpcs.xml

Copy link
Copy Markdown
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
Copy Markdown
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.

@recanman recanman left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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