friends-of-presta / fop_console

Prestashop Module providing a set of shell/terminal commands for developers (PrestaShop 1.7.5+)
Academic Free License v3.0
84 stars 35 forks source link

Fix code quality tools #233

Closed tom-combet closed 2 years ago

tom-combet commented 2 years ago

Fixes #150 (and closes #203?).

Fixed issues: • PHP-CS-Fixer workflow step wasn't using our custom configuration file. • PHPStan was old... very old... • GrumPHP was overwriting our level setting and checking dot files (where PHPStan doesn't).

nenes25 commented 2 years ago

Good job :+1: ! Do we need to check an other thing than that the automatic workflows are good ?

SebSept commented 2 years ago

I think we need to test on our machines.. I'd like to do it.

tom-combet commented 2 years ago

Yes, every reviewer should check on his side if GrumPHP works fine by running composer fop_check. You should have an "All Good" output.

SebSept commented 2 years ago

I will also make a commit in my ide

tom-combet commented 2 years ago

So you reported than grumphp overrides the default level, good catch. Maybe we can add a note in phpstan.neon about that. (and also in grumphp).

I already wrote a comment in grumphp configuration file. As it concerns only GrumPHP execution, I don't think it's necessary to add a note in phpstan configuration. Did you mean add a comment there too or did I misunderstand?

And that .dotfiles option also need to be specified.

Same here.

SebSept commented 2 years ago

I already wrote a comment in grumphp configuration file. As it concerns only GrumPHP execution, I don't think it's necessary to add a note in phpstan configuration. Did you mean add a comment there too or did I misunderstand?

Yes, that's what I mean. Someone may change the value in in phpstan.neon.dist then run fop_check or commit.

tom-combet commented 2 years ago

Ah ok I see, I'll add this 👍

SebSept commented 2 years ago

can anyone else confirm it's ok with their config ? @nenes25, @okom3pom, @jf-viguier

SebSept commented 2 years ago

I've just re tested, work fine both on command line and inside phpstorm :)

SebSept commented 2 years ago

can anyone else confirm it's ok with their config ? @nenes25, @okom3pom, @jf-viguier

ping

SebSept commented 2 years ago

You "just" need to resolve the conflicts @tom-combet .

tom-combet commented 2 years ago

@SebSept Done, thanks for validating it!