Silverstripe-framework: SS5: Better PSR-(1)2 conformance

Created on 11 Apr 2018  路  11Comments  路  Source: silverstripe/silverstripe-framework

Affected Version

5.x-dev

Description

A set of PSR-2 rules are ignored for framework, specifically because fixing them would require breaking changes to code. These exclusions are necessary for 4.x but should all be fixed in 5.x.

Edited: We will likely be aiming to adopt PSR-12 once it is an accepted PSR.

Steps to Reproduce

git clone https://github.com/silverstripe/silverstripe-framework.git -b=master
cd silverstripe-framework
phpcs --standard=PSR12 ./src/ ./tests/

many errors :)

Todo-list

Eliminate all PSR* exclusions in https://github.com/silverstripe/silverstripe-framework/blob/4.0/phpcs.xml.dist for master branch.

Pull requests

affectv4 affectv5 changmajor efforhard impacmedium rfaccepted typenhancement
Source
picture tractorcow
🎉1

All 11 comments

I've made a start at #7993

robbieaverill picture robbieaverill on 11 Apr 2018

I've been doing quite a lot of class renaming, getting rid of all classes with _ in them, as well as SS prefixed classes.

tractorcow picture tractorcow on 15 Apr 2018

Branches:

tractorcow picture tractorcow on 15 Apr 2018
🎉1

@tractorcow it might be worth thinking about PSR-12 for SS5 on top of PSR-2 (it claims to augment rather than replace PSR-1 and 2), assuming it gets accepted by PHP-FIG before SS5 is released. I notice you've voted unanimously in favour of all suggestions on behalf of SilverStripe for it

robbieaverill picture robbieaverill on 6 Sep 2018

Yeah, PSR-12, Slevomat, PHP 7.2+ or even PHP 7.3+ support would get a :+1: from me in principle.

sminnee picture sminnee on 18 Oct 2018
👍1

The good news is that a lot of the slevomat standards are "fixable" with phpcbf, e.g. PHP return type hinting etc

robbieaverill picture robbieaverill on 18 Oct 2018

Hey @tractorcow, with context of your WIP branches above and RFC: Rename all classes with underscores to be PSR-2 compliant, do you want to revive those branches and make PRs for them? Or should whoever picks up #8636 pick your branches up and roll with those?

robbieaverill picture robbieaverill on 9 Jan 2019

I've changed this to rfc/accepted, with me @sminnee and @tractorcow in support. I find it unlikely that the rest of the core team would be against this as well, will update to reference PSR-12 under the assumption that it moves from draft to accepted before this ticket is picked up.

Also worth noting that a PSR-12 codesniffer standard is already being developed 馃帀

robbieaverill picture robbieaverill on 9 Jan 2019

Sorry @robbieaverill I'm likely not going to be able to contribute the time needed to do such a large upgrade sorry. =( I suggest someone use my branches as inspiration, and just repeat it on the latest branches.

tractorcow picture tractorcow on 10 Jan 2019
👍1

I'm happy for us to aim for PSR-12 compliance but it shouldn't be done at any cost. I can see that the PSR-12 standard is much more detailed and prescriptive than the previous standards. The standard looks good, though

dhensby picture dhensby on 27 Feb 2019
👍1

Rule of thumb: that any API changes introduced by adopting the PSR should be handled by silverstripe-upgrader. If silverstripe-upgrader can't reliably do this, then the work's not likely to be worth it.

sminnee picture sminnee on 27 Feb 2019
Was this page helpful?
0 / 5 - 0 ratings

Related issues

Martimiz picture Martimiz  路  5Comments
maxime-rainville picture maxime-rainville  路  6Comments
icecaster picture icecaster  路  3Comments
dnsl48 picture dnsl48  路  6Comments
kinglozzer picture kinglozzer  路  6Comments