Skip to content

Add return type declaration for "equals()" method #88

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

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

jacek-foremski
Copy link

Adds return type declarations where applicable.

I required PHP 7.2, because it's the last version still in active support. This of course can be changed based on how maintainers prefer it.

One of the two PRs needed to fix #86

Copy link
Member

@mnapoli mnapoli 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 to me, you will have to rebase following the merge of #87 (there is a small conflict).

@jacek-foremski
Copy link
Author

Done

@mnapoli
Copy link
Member

mnapoli commented Apr 23, 2019

Damn I didn't think about this before now but adding return types is a BC break in case anyone has extended the class.

See https://3v4l.org/4DKvY for example.

However the equals() method is declared as final, which is the one you are most interested about right? We could add the return type only to this method.

@jacek-foremski jacek-foremski changed the title Add return type declarations Add return type declaration for "equals()" method Apr 23, 2019
@jacek-foremski
Copy link
Author

Ok, I reduced the scope to equals() method only since this is what interests me the most

@mnapoli
Copy link
Member

mnapoli commented Apr 23, 2019

Thank you, let's do this!

@mnapoli mnapoli merged commit e00b7b3 into myclabs:master Apr 23, 2019
@jacek-foremski jacek-foremski deleted the add_return_types branch April 23, 2019 12:04
@@ -87,7 +87,7 @@ public function __toString()
*
* @return bool
*/
final public function equals($variable = null)
final public function equals($variable = null): bool

Choose a reason for hiding this comment

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

What about adding other return types?

Copy link
Author

Choose a reason for hiding this comment

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

Please read the discussion above (basically it would be a BC break)

Choose a reason for hiding this comment

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

Ah, okay. Still working on my first cup of ☕️

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.

Compatibility with php-ds/ext-ds
4 participants