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

!!!TASK: Add PHP 7.0 scalar type hints to method arguments and return types #999

Merged
merged 1 commit into from Dec 27, 2017

Conversation

albe
Copy link
Member

@albe albe commented Jun 17, 2017

This is only breaking for classes implementing LockStrategyInterface, whose signature now requires scalar typehints.

@mention-bot
Copy link

@albe, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kdambekalns, @kitsunet and @radmiraal to be potential reviewers.

@albe albe changed the title TASK: Add PHP 7.0 scalar type hints to method arguments and return types !!!TASK: Add PHP 7.0 scalar type hints to method arguments and return types Jun 17, 2017
Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

Looks good (by reading)

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

Looks good, but must not be merged until 3.3 has been branched, since it's a breaking change. sigh

@@ -132,7 +132,7 @@ protected function tryToAcquireLock($exclusiveLock)
* @param boolean $exclusiveLock
* @throws LockNotAcquiredException
*/
protected function applyFlock($exclusiveLock)
protected function applyFlock(bool $exclusiveLock)
Copy link
Member

Choose a reason for hiding this comment

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

While we are at it, and since void as return type is only supported in PHP 7.1, why not add the missing @return annotation here? ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call

@albe
Copy link
Member Author

albe commented Jul 19, 2017

I was thinking of doing a b/c version of this and the other PR if I still get it done before the release. That way we hopefully have "as much as possible typehinting" in the release without breaking stuff and can then add breaking typehint in interfaces and possibly even strict_types for next major

@kitsunet kitsunet merged commit c2211b6 into neos:master Dec 27, 2017
@kitsunet kitsunet deleted the scalar-typehints-utility-lock branch December 27, 2017 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants