CS fix for #5245 #5391

Merged
merged 2 commits into from Nov 1, 2013

Projects

None yet

2 participants

@stefanotorresi
Contributor

The fixes done during #5245 merge had to be, well... fixed! :p

@stefanotorresi
Contributor

ha! if cs fix is applied to the test classes (which is what this PR does), the tests fail because the fixer alters the argument of the assertions.

possible solutions:

  • revert cs fix on tests too (this pr did revert it only for the test assets), and then add the two classes to the .php_cs ignore list.
  • update the test assets to work with cs-fixed tests.

i'll wait for comments.

@stefanotorresi stefanotorresi update .php_cs
some reflection tests have to be escluded from cs fixing otherwise
assertions arguments will be affected.
49b5e90
@weierophinney
Member

When I run php-cs-fixer after these changes, it still highlights FunctionReflectionTest.php, MethodReflectionTest.php, and TestAsset/closures.php. The tests still run after updating those, however. Shall I go ahead and apply those changes and finish the merge?

@stefanotorresi
Contributor

Hmm that's odd. Have you tried running it via check-cs.sh ?

@weierophinney
Member

@stefanotorresi Yes, of course. And those are the files it highlights -- tailing spaces and function_declaration errors on the first 2, and braces and function_declaration errors on the last.

@weierophinney
Member

@stefanotorresi actually... I have a script that goes and runs php-cs-fixer on any files that have changed from the branch I merged against. Evidently, php-cs-fixer does not consult the .php_cs file when you do that, or ignores it when the argument is a file and not a directory. We're good to go here.

@weierophinney weierophinney added a commit that referenced this pull request Nov 1, 2013
@weierophinney weierophinney Merge branch 'hotfix/5391' into develop
Close #5391
cdb4ab7
@weierophinney weierophinney merged commit 49b5e90 into zendframework:develop Nov 1, 2013

1 check failed

default The Travis CI build failed
Details
@weierophinney weierophinney was assigned Nov 1, 2013
@stefanotorresi
Contributor

ok then :)
for the record, it is exactly as you said: php-cs-fixer only evaluates .php_cs if it's contained in the directory passed as an argument, so running it on single files or subdirectories won't work.

@stefanotorresi stefanotorresi deleted the stefanotorresi:cs-fix/5245 branch Nov 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment