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

Camel case vars #128

Merged
merged 1 commit into from Jul 3, 2019
Merged

Camel case vars #128

merged 1 commit into from Jul 3, 2019

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Jul 1, 2019

I noticed there was no sniff enabled to force camelCase variable names.

I cannot create issue to ask so I created this PR. Is there a reason for absence of such rule or is this PR actually viable? Eg. methodNames() or ClassNames are required to be camelCased.

@simPod simPod requested a review from a team as a code owner July 1, 2019 08:21
@SenseException
Copy link
Member

Should the drop of PHP 7.1 put into another PR instead if this one? Or was this necessary to be able to introduce the new rule?

@malarzm
Copy link
Member

malarzm commented Jul 1, 2019

This can now be rebased atop new master as #127 was merged

@simPod
Copy link
Contributor Author

simPod commented Jul 1, 2019

This was necessary for CI to pass as it was not merged at the time of opening this PR. Rebasing now.

@carusogabriel carusogabriel added this to the 7.0.0 milestone Jul 2, 2019
Copy link
Contributor

@carusogabriel carusogabriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to note: DBAL, for example, has protected properties with a leading underscore:

FILE: lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php
-------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------
 38 | ERROR | Protected member variable "_conn" must not contain a leading underscore
    |       | (Squiz.NamingConventions.ValidVariableName.PublicHasUnderscore)
 45 | ERROR | Protected member variable "_platform" must not contain a leading underscore
    |       | (Squiz.NamingConventions.ValidVariableName.PublicHasUnderscore)
-------------------------------------------------------------------------------------------------------------------------------------------------------

changing that might result in a BC, but we can ignore this new rule there, the same way we do with ODM and classes names conventions:

https://github.com/doctrine/mongodb-odm/blob/a50624c38534a3f238353c83c12fb268eee61281/phpcs.xml.dist#L30-L33

@lcobucci lcobucci self-assigned this Jul 3, 2019
@lcobucci lcobucci merged commit e74b585 into doctrine:master Jul 3, 2019
@simPod simPod deleted the camel-case-vars branch September 16, 2019 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants