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

LongVariable rule should apply on private variables #527

Closed
duncancumming opened this issue Oct 12, 2017 · 6 comments
Closed

LongVariable rule should apply on private variables #527

duncancumming opened this issue Oct 12, 2017 · 6 comments
Milestone

Comments

@duncancumming
Copy link

Currently the LongVariable rule skips private variables:
https://github.com/phpmd/phpmd/blob/master/src/main/php/PHPMD/Rule/Naming/LongVariable.php#L55

The ShortVariable rule doesn't do the same.
https://github.com/phpmd/phpmd/blob/master/src/main/php/PHPMD/Rule/Naming/ShortVariable.php#L54

This isn't documented. Can we get rid of this?

@adamcameron
Copy link

I could not agree more. We still need to read private variable names just as much as the public / protected ones.

@ravage84
Copy link
Member

ravage84 commented Oct 12, 2017

Refers to change from: #16

@ravage84
Copy link
Member

@duncancumming and @adamcameron are you interested in putting up a PR for making this a configurable as I mentioned in my comment? (Hacktoberfest, anybody?

@ravage84 ravage84 added this to the 2.7.0 milestone Oct 12, 2017
@adamcameron
Copy link

Yeah Dunc and I (who are colleagues) were discussing this. We didn't want to charge in and make pull reqs just cos we felt like stuff ought to be different without getting a nod from the stewards of the app though.

Also... never made a pull req to this project. you got some dev guidelines or something we ought to eyeball first? We'd also need to get it cleared with our boss, but that shouldn't be too hard.

@ravage84
Copy link
Member

We are soemwhat light on guidelines, but take this for starters:
https://github.com/phpmd/phpmd/blob/master/CONTRIBUTING.md

Also, have a look at similar PRs:
#159
#197
#198

If you still have trouble, write me via email or Skype (same ID).

@adamcameron
Copy link

OK, I've run it up the flagpole at work. Will revert shortly.

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

No branches or pull requests

3 participants