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

Remove underscores from private/protected members names #3007

Conversation

mateuszsip
Copy link

Closes #3004

@mateuszsip mateuszsip force-pushed the codestyle/remove-underscores-from-members-names branch 3 times, most recently from 0a06bbe to 27c3af7 Compare January 31, 2018 03:55
@mateuszsip mateuszsip force-pushed the codestyle/remove-underscores-from-members-names branch from 27c3af7 to 2c1cf8a Compare January 31, 2018 04:05
@Majkl578
Copy link
Contributor

Restarted the failing build. Although not a blocker, can you please look into the CS issues? We currently have a rule that modified code should follow Doctrine CS (even if rest of the code doesn't yet).
We should also consider a BC layer for master in separate PR.
Otherwise 👍.

@morozov
Copy link
Member

morozov commented Jan 31, 2018

@Majkl578 I believe we could ignore the code style issues for massive automated changes like this as long as they don't introduce new issues. Or maybe whitespace/alignment issues could be fixed here as well.

What kind of a BC layer do you have in mind? For protected methods, we could have the deprecated old methods which invoke the new ones. But what about properties?

@Ocramius
Copy link
Member

Benefits here are too marginal to consider the patch, compared to massive BC concerns. Closing as won't fix.

@Ocramius Ocramius closed this Jan 31, 2018
@Ocramius Ocramius removed this from the 3.0.0 milestone Jan 31, 2018
@Ocramius Ocramius self-assigned this Jan 31, 2018
@Majkl578
Copy link
Contributor

Majkl578 commented Feb 2, 2018

How about reconsidering what really needs to be protected and for those provide final protected getter (and make fields private)?

@Ocramius
Copy link
Member

Ocramius commented Feb 2, 2018

@Majkl578 anything that we can make final probably should be, but we should do that before considering any style changes. That is a design change :-)

@morozov
Copy link
Member

morozov commented Feb 2, 2018

Besides the above, we still can fix private methods and properties and protected methods and properties in the non-abstract test classes.

As for private/protected as such, there's also lack of consistency across the codebase. For instance, MysqliConnection::_conn if private but OCI8Connection::dbh is protected.

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

Successfully merging this pull request may close these issues.

4 participants