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

Adopt PSR-12 for return types #163

Merged
merged 1 commit into from
Mar 20, 2020
Merged

Conversation

carusogabriel
Copy link
Contributor

@carusogabriel carusogabriel commented Mar 19, 2020

PSR-12 is an extension of the PSR-2 set of rules
that aims to standardize the code style.

Doctrine, so far, hasn't adopted 100% off it, and this PR proposes to start to migrate our rules
to PSR-12.

The first one is the return types. As described under the section 4.5,

The colon and declaration MUST be on the same line as the argument list closing parenthesis with no spaces between the two characters.

where the two characters are the closing parenthesis ) and colon :.

For context on why this was voted in the past, see #9.

[PSR-12](https://www.php-fig.org/psr/psr-12) is an extension of the PSR-2 set of rules
that aims to standardize the code style.

Doctrine, so far, hasn't adopted 100% off it, and this PR proposes to start to migrate our rules
to PSR-12.

The first one is the return types. As described under the section [4.5](https://www.php-fig.org/psr/psr-12/#45-method-and-function-arguments),
they SHOULD NOT be a blank space between the return type and colon (`:`).

For context on why this was voted in the past, see #9.# Please enter the commit message for your changes. Lines starting
@carusogabriel carusogabriel added this to the 8.0.0 milestone Mar 19, 2020
@carusogabriel carusogabriel requested a review from a team as a code owner March 19, 2020 22:42
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course!

@morozov
Copy link
Member

morozov commented Mar 19, 2020

As described under the section 4.5,
they SHOULD NOT be a blank space between the return type and colon (:)

For the record, I don't see the requirement as it's worded in the PR description. What I see is:

The colon and declaration MUST be on the same line as the argument list closing parenthesis with no spaces between the two characters.

Is this what you @carusogabriel are referring to?

Copy link
Member

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The colon and declaration MUST be on the same line as the argument list closing parenthesis with no spaces between the two characters.

Two characters here are colon and closing parenthesis. No space between them.

Checks out.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hell yeah!

@carusogabriel
Copy link
Contributor Author

carusogabriel commented Mar 20, 2020

I've updated the description, thanks for the review everyone!

I'll let this PR open for a week so others can vote as well.

@simPod
Copy link
Contributor

simPod commented Mar 20, 2020

Hi, is Doctrine going to strictly follow PSR12? IMO PSR12 was a nice step forward but is still far behind this CS.

As for myself, I preferred space around colon on both sides and that's one of things that I fancied on Doctrine CS.

@ostrolucky
Copy link
Member

Yes, our goal is to follow PSR12

@Majkl578
Copy link
Contributor

Huge 👎 from me as the original author of Doctrine CS. This is one of the advantages of Doctrine CS currently.

@carusogabriel
Copy link
Contributor Author

Hi, is Doctrine going to strictly follow PSR12?

That's is the idea. By the same way we follow PSR-2 I'm proposing to follow PSR-12. My two main cents:

  • Consistency with other projects and PSR
  • Easy to onboard new contributors that are used to PSR-12

@malarzm
Copy link
Member

malarzm commented Mar 20, 2020

I'll add that prolly we'll deviate somewhere from PSR-12 but IMO the colon thing is not worth deviating

@alcaeus
Copy link
Member

alcaeus commented Mar 20, 2020

For people that want to keep the space, you can always enforce that rule in your own standard. Doctrine CS is meant to be a strict set of rules based on PSR, and we encourage people to adjust and remove rules they don't agree with, or add their own rules for added checks.

@greg0ire greg0ire merged commit 7e7aff6 into master Mar 20, 2020
@greg0ire greg0ire deleted the rules/adopt-psr-12-for-return-types branch March 20, 2020 12:51
@greg0ire
Copy link
Member

Thanks @carusogabriel !

@alcaeus
Copy link
Member

alcaeus commented Mar 20, 2020

Thanks @carusogabriel!

@morozov
Copy link
Member

morozov commented Mar 20, 2020

As for myself, I preferred space around colon on both sides and that's one of things that I fancied on Doctrine CS.

Huge 👎 from me as the original author of Doctrine CS. This is one of the advantages of Doctrine CS currently.

Although I personally like the enforced whitespace more, I voted yes because I believe that standardization is more important than the personal taste. Furthermore, the latter will adapt very soon.

@greg0ire
Copy link
Member

Yeah I hated PSR-2 for the first 2 hours I used it. Once past that point, it's been a pleasure.

@kirya-dev
Copy link

Yeah, nice job

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

Successfully merging this pull request may close these issues.