CS: all methods must declare visibility #44543
Conversation
To not break BC, these have now all been set to `public`, but a careful review of the visibility for the (non-test) methods is recommended.
jrfnl
requested review from
ntwb,
nerrad,
ajitbohra and
dmsnell
as code owners
github-actions
bot
added
the
First-time Contributor
jrfnl
removed
the
First-time Contributor
There was a problem hiding this comment.
For whoever reviews this, the changes seem fine for the block parser but I can't speak for the other files.
@jrfnl can you share what led you to these files specifically? it would be nice to understand that and see if this couldn't be part of a more coordinated effort to have the computer rewrite code.
it would surely be easier for some (myself included) if these were split into separate PRs that touch a related subgroup of modules. as it stands we need everyone on board to examine the changes, or find someone confident enough to review everything, which even for a small change like this can take more focus time than it seems like it ought.
See: #18502 (comment)
Well, considering I've been mopping up after Gutenberg after every code merge to Core, I'd appreciate the courtesy of people spending a little time to get this fixed once and for all. I've split my PRs to mostly only contain one type of change, if someone wants to split it out even further, go right ahead, I won't stop you. |
Mamaduka
added
Gutenberg Plugin
What?
Adds missing visibility modifiers to class methods.
Why?
Visibility expresses whether a method is allowed to be used from outside of the class/from within child classes. It should always be expressly set to clarify intend of how the code should be used.
How?
To not break BC, these have now all been set to
public, but a careful review of the visibility for the (non-test) methods is recommended.