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

[ADD] PHP-CS-Fixer #159

Merged
merged 1 commit into from
Sep 8, 2016
Merged

Conversation

Th3Mouk
Copy link
Contributor

@Th3Mouk Th3Mouk commented Oct 21, 2015

Proposition

In order to fix easily code style convention, many bundles now use PHP-CS-Fixer.

The .php_cs file allow configuration on your needs, actually i've include the same as Sonata Project.

The use is really simple, when you're developing, type make cs or make cs_dry_run, thanks to Makefile, he will correct all the files with defined rules.

The actual configuration manage, too automatically, headers files of the bundle.

If this solution concurs for you, you're free to modify after merge the configuration.

Cheers 🍻

@DavidBadura
Copy link
Contributor

the change assertEquals to assertSame breaks the build.

/*
* (c) 2011 SimpleThings GmbH
*
* @package SimpleThings\EntityAudit
* @author Benjamin Eberlei <[email protected]>
* @author Andrew Tch <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong

if we use the header, please move the author above the class name and check the other files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Oct 21, 2015

Yes the php_unit_strict change the assertTest, i just removed it

@Th3Mouk Th3Mouk force-pushed the php-cs-fixer branch 2 times, most recently from 9c255bb to 5dc0ba5 Compare October 21, 2015 10:53
@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Oct 21, 2015

@DavidBadura @OskarStark Any other mistakes ?

@Th3Mouk Th3Mouk force-pushed the php-cs-fixer branch 2 times, most recently from 77477d5 to 1f045b0 Compare October 21, 2015 16:23
@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Nov 5, 2015

@DavidBadura ok for merge ?

@OskarStark
Copy link
Member

Looks ok to me 👍

@DavidBadura
Copy link
Contributor

it looks good to me. the probleme is, after we merge this all other PR's have then merge conflicts. maybe we should wait here?

@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Nov 10, 2015

The solution is to merged with using 'my branch' and ignoring upstream changes (if they're only relative to CS) and at the end type make cs command then commit 😉

All bundles had this problem with old PR's, when they changes testing or coding styles, but it's a problem that the time doesn't solve.

@OskarStark
Copy link
Member

btw i would keep the header the same everywhere and add special authors above the class declaration. otherwise anybody will have diffs, if he execute make cs...

what do you think?

@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Jan 6, 2016

@OskarStark in the header I put only the maintainer or co-funder, and yes special authors must be on the class declaration. The header is only relative to bundle licence. But isn't the case here in my memory

@OskarStark
Copy link
Member

OK @Th3Mouk 👍

;

return Symfony\CS\Config\Config::create()
->level(Symfony\CS\FixerInterface::SYMFONY_LEVEL)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just prefer the PSR2_LEVEL. The SYMFONY_LEVEL has some fixers I don't like.

@smoench
Copy link
Contributor

smoench commented Jan 7, 2016

@Th3Mouk Could you please revert the changes from php-cs-fixer fix-run. I must add some changes to the header comment on my own and a git rebase would be easier. Thanks in advance.

@bendavies
Copy link
Contributor

@smoench i'm going to pick this up. Do you have a preference of cs config?

@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Aug 19, 2016

As you want, i can modify too if necessary

@bendavies
Copy link
Contributor

@Th3Mouk thanks!
if you could rebase on master and address the other comments that would great

@Th3Mouk Th3Mouk force-pushed the php-cs-fixer branch 2 times, most recently from 05d2fd1 to c68adb2 Compare September 2, 2016 10:38
@Th3Mouk
Copy link
Contributor Author

Th3Mouk commented Sep 2, 2016

@smoench @bendavies it seems good

@DavidBadura
Copy link
Contributor

Thank you! :)

@DavidBadura DavidBadura merged commit b5be02a into sonata-project:master Sep 8, 2016
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.

5 participants