-
-
Notifications
You must be signed in to change notification settings - Fork 261
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
[ADD] PHP-CS-Fixer #159
Conversation
the change |
/* | ||
* (c) 2011 SimpleThings GmbH | ||
* | ||
* @package SimpleThings\EntityAudit | ||
* @author Benjamin Eberlei <[email protected]> | ||
* @author Andrew Tch <[email protected]> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Yes the |
9c255bb
to
5dc0ba5
Compare
@DavidBadura @OskarStark Any other mistakes ? |
77477d5
to
1f045b0
Compare
@DavidBadura ok for merge ? |
Looks ok to me 👍 |
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? |
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 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. |
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 what do you think? |
@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 |
OK @Th3Mouk 👍 |
; | ||
|
||
return Symfony\CS\Config\Config::create() | ||
->level(Symfony\CS\FixerInterface::SYMFONY_LEVEL) |
There was a problem hiding this comment.
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.
@Th3Mouk Could you please revert the changes from |
@smoench i'm going to pick this up. Do you have a preference of cs config? |
As you want, i can modify too if necessary |
@Th3Mouk thanks! |
05d2fd1
to
c68adb2
Compare
c68adb2
to
8d6cf45
Compare
8d6cf45
to
a307241
Compare
@smoench @bendavies it seems good |
Thank you! :) |
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
ormake cs_dry_run
, thanks toMakefile
, 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 🍻