You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Some of the checks are inconsistent. A few begin with check and others don't. Ideally we'd make the names short but sweet. This would require a lot of changes in the config files, but it'd end with nicer configuration values.
At the beginning there was no "checkXXX", I started adding them to order the methods in the IDE. But if we manage to separate the checks (or some of them) from the main code, we should adopt shorter names.
Good idea. I think the best thing to do is each check is passed the Reporter and the token stack etc. This way it's able to decide how to handle each check individually.
We could always run the names through a converter, so camelCase could still be default, but it could auto convert snake_case etc? This just allows for easy mistakes etc.
Tests that use a regexp should change from constantNaming to constantNamePattern, so you know it's an expected format.
Where we've reduced works like statement to stmt we should undo that. Its much more readable to have spaceAfterControlStatement, although it's longer, it's more verbose.
We should drop the check from keys like checkProhibitedFunctions so just prohibitedFunctions.
showTODOs could just be showTodo for camelCase-ness.
My thoughts are for readability and verbosity, but still keeping them short where possible. What do you think?
Activity
jbrooksuk commentedon Aug 29, 2014
Also, this would have to be marked as a script breaking change - unless we can add aliases?
jbrooksuk commentedon Sep 3, 2014
What do you think @tchule?
tchule commentedon Sep 8, 2014
Agree.
At the beginning there was no "checkXXX", I started adding them to order the methods in the IDE. But if we manage to separate the checks (or some of them) from the main code, we should adopt shorter names.
jbrooksuk commentedon Sep 8, 2014
Would you agree with names such as
unusedVariables
andunusedFunctionParameters
?tchule commentedon Sep 8, 2014
Yes, looks good to me.
tchule commentedon Sep 8, 2014
On a slightly different but still related subject, I've updated an old task on the google code site : https://code.google.com/p/phpcheckstyle/issues/detail?id=1
I've started to try to implement something but I'm not really decided yet on the proper way to do this.
jbrooksuk commentedon Sep 8, 2014
Good idea. I think the best thing to do is each check is passed the Reporter and the token stack etc. This way it's able to decide how to handle each check individually.
Sent from my iPhone
jbrooksuk commentedon Sep 9, 2014
Perhaps even using under scores in names?
unused_variables
line_length
Easier to read?
jbrooksuk commentedon Sep 9, 2014
We could always run the names through a converter, so camelCase could still be default, but it could auto convert snake_case etc? This just allows for easy mistakes etc.
jbrooksuk commentedon Sep 22, 2014
@tchule just a quick check that you're happy with the suggested naming conventions before I go and change them?
tchule commentedon Sep 22, 2014
Personnaly I prefer the camelCase version, klike you proposed "unusedVariables" and "unusedFunctionParameters". But I'm OK with both.
jbrooksuk commentedon Sep 22, 2014
Ok, well we can easily accept both - a quick conversion would fix it - but we in the documentation and example we would always use camelCase?
tchule commentedon Sep 22, 2014
Yes, this would be perfect.
jbrooksuk commentedon Sep 24, 2014
@tchule here's what I'm thinking:
regexp
should change fromconstantNaming
toconstantNamePattern
, so you know it's an expected format.spaceAfterControlStatement
, although it's longer, it's more verbose.check
from keys likecheckProhibitedFunctions
so justprohibitedFunctions
.showTODOs
could just beshowTodo
for camelCase-ness.My thoughts are for readability and verbosity, but still keeping them short where possible. What do you think?
Also, they read better in snake_case too:
constant_naming_pattern
space_after_control_statement
prohibited_functions
show_todos
:)
2 remaining items