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.

Sign up for GitHub

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

Jump to bottom

fix: parameter and return type of HubInterface #849

Merged
merged 1 commit into from
Jul 11, 2019
Merged

fix: parameter and return type of HubInterface #849

merged 1 commit into from
Jul 11, 2019

Conversation

B-Galati
Copy link
Contributor

@B-Galati B-Galati changed the title fix #845 - fix parameter and return type fix: parameter and return type of HubInterface Jul 10, 2019
@ste93cry ste93cry added this to the 2.1 milestone Jul 10, 2019
ste93cry
ste93cry approved these changes Jul 10, 2019
View reviewed changes
Copy link
Collaborator

@ste93cry ste93cry left a comment

Choose a reason for hiding this comment

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

I don't know why I convinced myself that this could not be done without breaking BC, but apparently it works because there was just one way to get it working right now and that way is the same as the change 🚀

@ste93cry
Copy link
Collaborator

There is one missing thing: can you please update the CHANGELOG?

@Jean85
Copy link
Collaborator

Jean85 commented Jul 11, 2019

@ste93cry that's probably because all the implementation that would be broken are possible only with PHP 7.4 where proper type widening (contravariance) is implemented: https://3v4l.org/EWG2S

So if we release this before PHP 7.4 we're good 😄

@B-Galati
Copy link
Contributor Author

@ste93cry that's probably because all the implementation that would be broken are possible only with PHP 7.4 where proper type widening (contravariance) is implemented: https://3v4l.org/EWG2S

It would be ok for the return type to use self as it is covariant but it's not for the parameter type to have self as it must be contravariant as per the RFC -> https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters.

@B-Galati
Copy link
Contributor Author

Changelog updated!

ste93cry
ste93cry requested changes Jul 11, 2019
View reviewed changes
Copy link
Collaborator

@ste93cry ste93cry left a comment

Choose a reason for hiding this comment

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

Really quick change, then LGTM 🚀

CHANGELOG.md Outdated Show resolved Hide resolved
@ste93cry ste93cry merged commit 3e3bc54 into getsentry:develop Jul 11, 2019
@ste93cry ste93cry mentioned this pull request Jul 11, 2019
Closed
@ste93cry ste93cry modified the milestones: 2.1, 2.2 Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jean85 Jean85 Jean85 approved these changes

@ste93cry ste93cry ste93cry approved these changes

Assignees
No one assigned
Labels
Type: Improvement
Projects
None yet
Milestone
2.2
Development

Successfully merging this pull request may close these issues.
3 participants
@B-Galati @ste93cry @Jean85