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

Jump to bottom

[WIP] psr 1, 2 and 4 #7731

Closed
wants to merge 12 commits into from
Closed

[WIP] psr 1, 2 and 4 #7731

wants to merge 12 commits into from

Conversation

ThaDafinser
Copy link
Contributor

Like written here: http://developer.piwik.org/guides/contributing-to-piwik-core
Make sure your code follows the PSR 1, PSR 2 and PSR-4 coding standards.

The php-cs-fixer addresses this problem and you can automatically check the coding standards + can fix them also automatically.

Just execute locally vendor\bin\php-cs-fixer fix and it will fix currently the core/API folder to the standards

@todo / problems:

  • it required php 5.3.6 (most php libs already do 5.3.23)
  • enable it for all piwik folders (i skipped it for now, to show you the benefit first and not create a too big diff)
  • enable more usefull fixers like long_array_syntax (like ordered_use)

Related #7186

@ThaDafinser ThaDafinser changed the title Feature/php cs fixer [WIP] psr 1, 2 and 4 Apr 21, 2015
@ThaDafinser
Copy link
Contributor Author

@diosmosis can u please help me, where i should put this line in the travis.sh

ThaDafinser@0a6cf2b

@apoco76
Copy link

apoco76 commented Apr 22, 2015

please remove this email from all Piwik groups, my husband died on March
7th. Thank you.

James Hester's Wife.

On Wed, Apr 22, 2015 at 3:45 AM, Martin Keckeis notifications@github.com
wrote:

@diosmosis https://github.com/diosmosis can u please help me, where i
should put this line in the travis.sh

ThaDafinser/piwik@0a6cf2b
ThaDafinser@0a6cf2b

—
Reply to this email directly or view it on GitHub
#7731 (comment).

Jay Hester

And we know that all things work together for good to them that love God,
to them who are the called according to his purpose. Romans 8:28

@mattab
Copy link
Member

mattab commented Apr 22, 2015

Hello @apoco76 - sorry to hear your loss. to remove the email from the Piwik group you need to go to https://github.com/piwik/piwik and then click "Unwatch" icon in the top

@ThaDafinser
Copy link
Contributor Author

@apoco76 i'm also sorry to hear that.

@mattab what about the PR? Wanted or unwanted to implement it?

mattab
mattab reviewed Apr 27, 2015
View reviewed changes
.php_cs.cache
@@ -0,0 +1 @@
a:3:{s:7:"version";s:49:"1.7:v1.7#6425aeb97ab921371182712a18c280d546e7769b";s:6:"fixers";a:21:{i:0;s:6:"braces";i:1;s:6:"elseif";i:2;s:8:"encoding";i:3;s:10:"eof_ending";i:4;s:19:"function_call_space";i:5;s:20:"function_declaration";i:6;s:11:"indentation";i:7;s:20:"line_after_namespace";i:8;s:8:"linefeed";i:9;s:17:"long_array_syntax";i:10;s:19:"lowercase_constants";i:11;s:18:"lowercase_keywords";i:12;s:21:"method_argument_space";i:13;s:12:"multiple_use";i:14;s:11:"parenthesis";i:15;s:15:"php_closing_tag";i:16;s:4:"psr0";i:17;s:9:"short_tag";i:18;s:25:"single_line_after_imports";i:19;s:15:"trailing_spaces";i:20;s:10:"visibility";}s:6:"hashes";a:13:{s:15:"ApiRenderer.php";i:1122214539;s:15:"CORSHandler.php";i:666994348;s:26:"DataTableGenericFilter.php";i:-1983359483;s:34:"DataTableManipulator\Flattener.php";i:848681176;s:36:"DataTableManipulator\LabelFilter.php";i:-527210726;s:47:"DataTableManipulator\ReportTotalsCalculator.php";i:-509427278;s:24:"DataTableManipulator.php";i:793745211;s:26:"DataTablePostProcessor.php";i:-1124517554;s:26:"DocumentationGenerator.php";i:-85202665;s:19:"Inconsistencies.php";i:-511824239;s:9:"Proxy.php";i:794139301;s:11:"Request.php";i:-669359659;s:19:"ResponseBuilder.php";i:1998418150;}}
Copy link
Member

Choose a reason for hiding this comment

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

maybe you can add .php_cs.cache to .gitignore?

@mattab
Copy link
Member

mattab commented Apr 27, 2015

Hi @ThaDafinser - anything that improves the Piwik sourcecode is welcome: it will make easier for contributors to get into it and contribute.

for sure we will merge your code improvements / PSR fixes.

(btw at this stage imho we don't need / want to add it to travis CI as it's easier to manually run it once in a while. )

@ThaDafinser
Copy link
Contributor Author

@mattab i updated the sources (no build check + gitignore)

Left problem is now PHP 5.3.3 https://travis-ci.org/piwik/piwik/jobs/60175240#L250

We can solve it only by higher the PHP requirement to 5.3.6....which is not wanted i think?

@ThaDafinser
Copy link
Contributor Author

Alternaive: i only add .php_cs without composer and execute the command local

@mattab mattab mentioned this pull request May 21, 2015
Closed
@PatchRanger
Copy link

@ThaDafinser @mattab Alternative is to skip php-cs-fixer for php 5.3.3 (see my pull-request to @ThaDafinser repository https://github.com/ThaDafinser/piwik/pull/1 and to piwik #8047). That seems to make all tests green - though I had no chance to check it because of merge conflicts that need to be resolved before Travis-CI had a chance to run the tests.

@PatchRanger PatchRanger mentioned this pull request Jun 6, 2015
Closed
@ThaDafinser
Merge pull request #1 from PatchRanger/feature/php-cs-fixer
bd3a71f 
Skip fabpot/php-cs-fixer for php 5.3.3
@mattab mattab mentioned this pull request Jun 25, 2015
Closed
@ThaDafinser
Copy link
Contributor Author

Closing (too old and some things are already in)

@ThaDafinser ThaDafinser mentioned this pull request Sep 3, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ThaDafinser @apoco76 @mattab @PatchRanger