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

Coding Standards: Add visibility to methods in src directory. #1713

Closed
wants to merge 1 commit into from

Conversation

costdev
Copy link
Contributor

@costdev costdev commented Sep 27, 2021

Coding Standards: Add visibility to methods in src directory.

"Visibility should always be declared"
See: https://make.wordpress.org/core/2020/03/20/updating-the-coding-standards-for-modern-php/

This commit applies the following sniffs to files in src:

  • Squiz.Scope.MethodScope
  • PSR2.Methods.MethodDeclaration
  • PSR2.Classes.PropertyDeclaration
  • Squiz.WhiteSpace.ScopeKeywordSpacing

For all methods, these now indicate public visibility to avoid breaking backwards compatibility.

Trac ticket: https://core.trac.wordpress.org/ticket/54177

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.

Ready for commit.

All methods which did not have an explicit visibility have now been made public.

This is the correct visibility as no visibility implicitly makes a method public and, even though some methods may have been marked as "private" in the docblock, as the methods did not have visibility before, making these explicitly private would be a backward compatibility break, so public is the right choice for all these methods.

@costdev costdev changed the title #54177 - Apply PHPCS fixes to src files. Coding Standards: Add visibility to methods in src directory. Oct 18, 2021
@hellofromtonya
Copy link
Contributor

Committed via changeset 51919.

@costdev costdev deleted the #54177-src branch September 19, 2022 21:04
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