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
Automatically reformat all source code in Piwik to follow updated coding standard #3771
Comments
For PHP coding standards, I'm leaning towards PSR0,1,2. |
Please do not automaticly update all php files to match any coding standards. That will cause merging problems for everyone having any unpushed changes to those files. And I have quite a lot of those in my refactoring branch... PSR 0/1/2 sounds good. It uses spaces instead of tabs. I don't like tabs ;) |
the big difference is the curly braces on the same line, isn't it? I guess it's a difference but Im still not keen to change this style, until the code is much clearer. Right now it would look messy if we changed to this style. Id like to refactor existing code and refactor a lot at some point! |
Replying to SteveG:
thanks for the reminder... We will specifically wait for your branch (as I believe you're the only one to have a big change in the works). looking forward to it! But we have to apply the changes automatically at some point, since it's time consuming to have spaces & style differences in the files. I think we'll keep tabs, since it has the advantage that you can change the width in IDE & it's what we're using in 95% of the code... Adding to ticket:
|
PSR-0/1/2 really sound great - yet I agree with matt that we should keep tabs instead of spaces. @matt, for the automatic reformatting it might be interesting to use https://github.com/fabpot/PHP-CS-Fixer - it applies PSR-0/1/2 automatically to all files in a directory (you can deactivate the indentation fix). Could be easier than doing it with PhpStorm. |
Cs-fixer uses regexes, not the tokenizer, so it sometimes makes mistakes. We would need to manually verify its changes. I don't mind the switch from tabs to spaces. There's no visual difference because we used ts=4, and I don't think the additional storage requirements will be noticed by users. Plus, iirc GitHub (or is it gitlab?) uses ts=2 in diffs, so it's distracting when tabs and spaces are both used for indentation. So +1 for eliminating tabs. Agree with not bulk updating all the files (at least not in master). Need a plan... |
I changed my mind, we should use full PSR 0/1/2 including white spaces instead of tabs (it seems more people like spaces these days). Also I'm hoping to use phpstorm rather than another tool, hope it will work. I plan to make this change some time in March (after 1.11 before 1.12 ?). |
I didn't add the unit test as it would be annoying. Otherwise, PSR conversion done! ae4b031 |
See also our follow up issue |
This task goal is to cleanup a bit since as the team has grown there has been some freedom in the style, but it's nicer for everyone to see/read one style.
This ticket is multi task:
I've got the profile in phpstorm and will commit all updated PHP files soon.
Any suggestion on how to improve the code & standards is always welcome.
The text was updated successfully, but these errors were encountered: