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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to consistent naming of `private` and `protected` class members #151

Merged
merged 4 commits into from Sep 11, 2018

Conversation

Projects
None yet
3 participants
@jonrandy
Contributor

jonrandy commented Sep 10, 2018

1. Objective

I noticed that we were randomly enforcing a naming convention of using an underscore prefix for private and protected class members. This should be enforced right across the plugin

2. Description of change

Renamed all affected class members so that they now have the required underscore prefix

3. Quality assurance

Re-tested all plugin functionality after making the changes

4. Impact of the change

Now easier to see at a glance whether a class member is public or not

5. Priority of change

Normal

6. Additional Notes

馃懢馃懢馃懢

@jonrandy

This comment has been minimized.

Show comment
Hide comment
@jonrandy

jonrandy Sep 10, 2018

Contributor

Other changes are also in here as it is a branch off of a previous branch that is awaiting review

Contributor

jonrandy commented Sep 10, 2018

Other changes are also in here as it is a branch off of a previous branch that is awaiting review

@guzzilar

@jonrandy @jacekstan prefixed _ is not recommended in PSR-2 which the one that Magento applied with.
https://www.php-fig.org/psr/psr-2/#42-properties

As we are trying to follow the platform code standard, I think we should just leave it without _ prefix.

(also, we might get into some issue when we submit out extension to their store, as Magento is kinda strict on coding-standard)

@guzzilar

just checked, that PSR-2 rule has been applied on only Magento 2.

@jonrandy jonrandy merged commit b6e31fe into 1-stable Sep 11, 2018

@jonrandy jonrandy deleted the 1-consistent-underscore-protected branch Sep 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment