Change to PSR-2?

Ok, @daniellienert and I checked, and while you can configure StyleCI a lot, it does not allow to be configured to match our current CGL. So it’s PSR-2 if we want to use that. I tried it with a repository of mine, and the suggested code looked horrible (to me). Sigh.

Anyway, talking about PHP-CS-Fixer, if my memory serves me right, @radmiraal has a config matching the Flow CGL for that. :smile:

I just wanted to note here that we voted on that topic (http://doodle.com/3xq2uvn4sa4d6tqu) and it was pretty much clear that we should/could switch to PSR-2 (fully, no exceptional rules).

I think we should all try to be not that emotional about the coding style. There are certainly advantages of using PSR-2:

  • Interoperability
  • Not having to switch your IDE to many different CGLs
  • New contributors for PHP code will have an easier time
  • We have less NIH
  • It’s one more standard we subscribe to, makes Flow less exotic
  • If we really target integrating more of Symfony at one point, it’s just natural to use the same CGL
  • And lastly: a custom CGL seems like a premium bikeshed case to me (http://bikeshed.com/)

Well, no-one mentioned that vote is closed, I still voted last night… :wink:

Anyway, if we have a decision, then we should close the Doodle, announce it and (sigh) use it.

I didn’t officially close the vote, exactly for hte reason that we didn’t have so much participation so far.

I’m also surprised that this discussion got so emotional - it’s just a few changes and we’ll get used to them in no time. What I personally find much more important are the technical implications. For example the fact that this will make backporting a whole lot more cumbersome because cherry picking to older branches won’t be possible with manual adjustments most of the time… But, well, I think we’ll have to live with that

What I don’t understand/know is, whether this means, if we switch to PSR-2, that we will also try to adapt those CGLs to TypoScript, HTML/Fluid, JS and SCSS or would it only mean changes for PHP?

Unless we update adjust all maintained releases with the style change. If it’s a straightforward process, this might be preferable over adjusting all backports. But depends I guess.

TBH I also dislike the PSR-2 and mainly for one reason: 4-Space indentation (well, that and the inconsistency in curly bracket usage on same/next line)

I never understood why you would want to use four sequential whitespace characters to indent stuff, when there’s a single tab character that does the same with one key/character. It just adds more bytes to your code.

Yes, some people like to make tab width something different from equal to 4 whitespaces, which will break things, but why not just come up with a standard tab width instead of avoiding tab completely just for the sake of it? Also, if your IDE is not configured to use 4-white spaces instead of tab, it will also break and mind the case where you manually indent (it’s done by people) and use one space too much/few. IMO it’s actually more error prone than single tab (ever had any issues with a YAML file not perfectly indented?).

So personally, I would not go with PSR-2, but from a Framework POV it makes a lot of sense to adopt a standard CGL.

For the record, I also never liked the style in PSR-2, mostly for the curly-braces-new-line syntax. It feels a bit like agreeing on using C++ coding guidelines for Ruby projects, but well …

I voted in favour of the change because it lowers the barrier for new contributors. I was reluctant, because it will be a hassle for existing users of Flow and Neos who will have to convert their code and probably even automatic checks (code sniffer) in Jenkins etc. It’s a little hassle and no real problem, so I think we can do it.

Awesome how we can be attached to a curly brace position :wink:

But this vote is not about aesthetic, it’s about standard and lowering the barrier. We, as a team/community, need to move to more open standard, and the PSR2 move, is easy and a nice first move … next step we can have more serious and code related discussion around PSR-7, …

My off topic note, in Go they provide go-fmt … an official automatic CGL enforcement … and it’s pretty nice to have all those package from all around the planet that use a consistent CGL.

Please let’s do it, it’s just about character position …

As far as I can see in the vote, it’s pretty much decided in favor of changing to PSR-2. I guess I will leave the doodle open until Monday or so. Then we can start planning how and for which branches we want to do this.

1 Like

Oh, sigh. :smile:

I guess we want to do it for master only, but merge hell lies ahead if we do.

Definitely. Changing all files in a minor or even patchlevel-release is a no-go IMO…
I guess we’ll just have to live with the merge hell for a while… I don’t know how exactly backporting will work on GH but maybe we can even add some magic that knows how to deal with the PSR change transparently

Definitely. Changing all files in a minor or even patchlevel-release is a no-go IMO…

Why? I mean this is a totally non functional change. Sounds strange but I don’t see a problem in doing that.

agreed – if this doesn’t introduce a bug, it’s actually no change from a user’s perspective.

2 Likes

I guess you’re right… That would make things a lot easier of course!

1 Like

Just FYI: I closed the poll and it looks like we ar going PSR-2. Lets plan that soonish, when we have defined our github workflow.

1 Like

For those who don’t want to search the full discussion: the above mentioned closed poll is at http://doodle.com/3xq2uvn4sa4d6tqu :wink:

Closing this topic because the core packages were converted to PSR-2 in the meantime.

1 Like