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

Next round: PSR-2 + php-cs-fixer #8704

Closed
wants to merge 4 commits into from
Closed

Next round: PSR-2 + php-cs-fixer #8704

wants to merge 4 commits into from

Conversation

ThaDafinser
Copy link
Contributor

Hello together,

i've done now really the smallest part to finally add php-cs-fixer to piwik.

How to install global: https://github.com/FriendsOfPHP/PHP-CS-Fixer#globally-composer

I extra left out following things:

Include it for all php folders - Why not?

You guys should activate it, when you think its a good time that not all PRs need to go through a merge "hell". Just change the exclude/include part in .php-cs-fixer

Do not add it to the build system and even not composer

As long as Piwik still support PHP 5.3.3 this is not possible. When Piwik at least requires PHP 5.3.6 we can add it then.

#7731
#8047
#7186
#7955

@mattab i think it would be a good idea to add this also to 2.15, since then merging/backporting from 3.x branch down would be easier?

My idea is to execute this by hand first from time to time and add more and more folder, until we have all. Then when also PHP 5.3.6 is required we add it to the build system

@mattab
Copy link
Member

mattab commented Sep 11, 2015

Hi @ThaDafinser I think (if i'm not mistaken) that if we want to fix the code (IMO it's nice to have!), we should do it rather quickly or we may risk a merge-hell with 3.0.0 and LTS branch. @tsteur any thought on this maybe?

@tsteur
Copy link
Member

tsteur commented Sep 11, 2015

yes would be good to do after merging 3.0 to master! I already had yesterday some troubles re some line ending changes when I merged master => 3.0

@mattab
Copy link
Member

mattab commented Sep 11, 2015

Just to be sure, do you suggest we should do the PSR fix only after 3.0.0 has become master branch, ie. in 2016?

@ThaDafinser
Copy link
Contributor Author

yes @mattab you are right: ASAP ...otherwise merging wont be fun, but with this change it will be much easier!

I just let it open for you guys, since you can choose a good moment - e.g. most (large) PRs are merged already....

@ThaDafinser
Copy link
Contributor Author

Fixes executed #8884 #8885 #8888 #8889

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

Successfully merging this pull request may close these issues.

None yet

3 participants