Skip to content
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

full (i think) PSR-2 standardized #2843

Closed
wants to merge 1 commit into from
Closed

full (i think) PSR-2 standardized #2843

wants to merge 1 commit into from

Conversation

SerafimArts
Copy link
Contributor

Standardized:

  • braces location
  • spaces instead of tabs
  • spacing between dots and brackets
    ...and a few other little things

@brunogaspar
Copy link
Contributor

wut?!

@SerafimArts
Copy link
Contributor Author

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md default php coding standard

Laravel 4 yet should only standard PSR-0 (and PSR-1)

@SerafimArts
Copy link
Contributor Author

Sorry for bad english =)

@andrewryno
Copy link
Contributor

Laravel is designed to only be PSR-1 compliant. See #1606, #2333, etc. (there have been loads of these issues).

@SerafimArts
Copy link
Contributor Author

This is very upsetting, so after "artisan migrate:make", "constroller:make" have to edit the source code to stick to the standard. Even the sources are not consistent places, although Laravel held in php-fig...

p.s. little did not understand how the above issues relate to the PSR-2 code style-guide: location brackets, spaces, quotes, etc.

@Anahkiasen
Copy link
Contributor

Nevertheless, I do agree it's kind of painful to have the generators output PSR2 invalid files, the source code is one thing, but Laravel shouldn't force coding standards in applications using it. That may sound extreme, and I'm always the first one to defend the adoption of PSR, so my opinion is to be weighted, but still.

@bencorlett
Copy link
Contributor

Woah @SerafimArts. I can tell a LOT of effort went into this.

Unfortunately, as part of contributing (and as part of you creating a pull request), it would have asked you have you read the CONTRIBUTING guidelines, in particular, with Pull Requests.

This has been discussed a few times. When you're modifying these sorts of file counts, it's only going to lead for disappointment as I can tell you've spent a lot of time here. This is exactly the reason we have contributing guidelines, so that these things can be discussed before you waste a lot of your time.


My honest opinion regarding the actual PR? Laravel's code looks prettier but at the end of the day I'd like to see PSR-2. However, with so many other open pull requests and issues, this will quickly become invalidated and impossible to merge in.

This decision is up to Taylor completely and it really doesn't matter which way he goes.

@philsturgeon
Copy link

In an ideal world everyone would use as many PSRs as possible, but the attitude displayed here is rather damaging to the purpose of the FIG and is spreading the wrong message.

Just because Laravel is part of the FIG, does not mean it_has_ to be PSR-2 compatible. There has never been a requirement for all projects to implement all PSRs, it is simply softly advised.

So, if Taylor knows this (which he does) and has chosen to ignore PSR-2 for his own reasoning (which he has) then I have to wonder what benefit this pull request adds. If Taylor wanted to change to PSR-2 he would run code sniffer. If he didn't want to switch to PSR-2 then he will close this PR.

This is very upsetting, so after "artisan migrate:make", "constroller:make" have to edit the source code to stick to the standard.

PHP Code Sniffer is great at taking care of this.

Should Laravel 4 be PSR-2? Now that some of the inconsistencies in Code Sniffer have been resolved maybe it would be more likely, but there are still more issues to be clarified like this one and at least 5 more that I know about.

If PSR-2 was a perfect standard that everyone loved I'd say "oh yeah, everyone should implement it all the time", but it's not so people have to respect the decisions made by projects.

@SerafimArts
Copy link
Contributor Author

Make it was not so difficult. Some regex replacements and checking the result. So if that commit will not work - I would not be too upset.

@taylorotwell
Copy link
Member

Laravel doesn't follow PSR-2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants