search

phpstan / extension-installer

Composer plugin for automatic installation of PHPStan extensions.
MIT License
426 stars 27 forks source link

Add PSR-12 Style on CodeSniffer ruleset. Exclude or customize conflict rules with PSR-12. #9

Closed niconoe- closed 5 years ago

niconoe- commented 5 years ago

This is what I had in mind with #8

The most important work is about the identation: the PSR-12 requires to have 4 spaces instead of tabs. If this is a rule you don't want because you prefer tabs, I can change the PR to exclude that rule and let the tabs. After all, what matters is consistency, not "tabs" or "spaces" endless war...

The other stuff is about the spaces before and after the braces on the class definition. Accoring to PSR-2: "Opening braces for classes MUST go on the next line, and closing braces MUST go on the next line after the body", that's why I reviewed some rules properties. It's better than excluding the rule.

Everything else is fine and follows the PSR-12.

I didn't know about the SlevomatCodingStandard, and they provide very good stuff! I might find a way to use some of their rules for my projects :smile:.

Anyway, if you don't want to accept all new changes but only a few, I can review the PR according to your coding rules.

ondrejmirtes commented 5 years ago

No, sorry, this is exactly what I haven’t got in mind. I don’t want to change formatting exactly, I more want to find functional issues in the code.

On Wed, 12 Jun 2019 at 15:22, Nicolas Giraud notifications@github.com wrote:

This is what I had in mind with #8 https://github.com/phpstan/extension-installer/issues/8

The most important work is about the identation: the PSR-12 requires to have 4 spaces instead of tabs. If this is a rule you don't want because you prefer tabs, I can change the PR to exclude that rule and let the tabs. After all, what matters is consistency, not "tabs" or "spaces" endless war...

The other stuff is about the spaces before and after the braces on the class definition. Accoring to PSR-2: "Opening braces for classes MUST go on the next line, and closing braces MUST go on the next line after the body", that's why I reviewed some rules properties. It's better than excluding the rule.

Everything else is fine and follows the PSR-12.

I didn't know about the SlevomatCodingStandard, and they provide very good stuff! I might find a way to use some of their rules for my projects 😄.

You can view, comment on, or merge this pull request online at:

https://github.com/phpstan/extension-installer/pull/9 Commit Summary

  • Add PSR-12 Style on CodeSniffer ruleset. Exclude or customize conflict rules with PSR-12.

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/phpstan/extension-installer/pull/9?email_source=notifications&email_token=AAAZTOG5OTUX62ED37ZMGJDP2D2C5A5CNFSM4HXI5U7KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GZB7FOQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZTODIH6RTVUNHIQTADWLP2D2C5ANCNFSM4HXI5U7A .

--

Ondřej Mirtes

niconoe- commented 5 years ago

Ok, I understand. PSR-12 is only about formatting so it totally doesn't fit with your thoughts. Do you prefer to not rely on that PSR and close this PR or are you interested in, even if it wasn't what you thought about?

To me, even if it's a small repository, it's never too early to bring coding standards ;).

ondrejmirtes commented 5 years ago

The repository currently has an enforced coding standard, don't worry ;)

Most activity is in phpstan/phpstan so any change will have a bigger impact there. If you want to contribute, I welcome you to send one new sniff at a time with related changes. But please, no PSR2/PSR-12, especially not indenting with spaces :)

niconoe- commented 5 years ago

Ok, so let's close this PR as it adding too much sniffs at a time and changes the indentation way. I'll have a look on phpstan/phpstan sources on the future. I can only contribute when I have some time allowed at work, and those "free-time" do not happen oftenly... That's why I looked at this project: only 1 source file was perfect to the time I can have on contributing :D.

Thanks again for your time.

ondrejmirtes commented 5 years ago

No problem 😊

On Wed, 12 Jun 2019 at 16:47, Nicolas Giraud notifications@github.com wrote:

Closed #9 https://github.com/phpstan/extension-installer/pull/9.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/phpstan/extension-installer/pull/9?email_source=notifications&email_token=AAAZTOEQHKB2IXJX7BVQ7YLP2ED6JA5CNFSM4HXI5U7KYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOR6A2OKA#event-2407638824, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZTOAPWH2QGIMW5WVHXNDP2ED6JANCNFSM4HXI5U7A .

--

Ondřej Mirtes