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

Typehints #208

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Typehints #208

wants to merge 8 commits into from

Conversation

tvdijen
Copy link
Contributor

@tvdijen tvdijen commented May 11, 2020

This is a good start into PHP 7.1 and PSR-12 compliancy.. Some Psalm-issues remain

@codecov-io
Copy link

codecov-io commented May 11, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@78ca6f0). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #208   +/-   ##
=========================================
  Coverage          ?   69.28%           
  Complexity        ?      402           
=========================================
  Files             ?        5           
  Lines             ?     1133           
  Branches          ?        0           
=========================================
  Hits              ?      785           
  Misses            ?      348           
  Partials          ?        0           

@tvdijen
Copy link
Contributor Author

tvdijen commented May 11, 2020

This is only half the work.. I took out the low hanging fruit, but the remaining Psalm-issues should be addressed

@robrichards
Copy link
Owner

@tvdijen I am just going to hang on to this until I am considering a 4.x branch. There are too many BC breaking changes. I will go through the type hint only change to see if that might be able to be applied. May also grab some of your changes that fix codecov as well.

@tvdijen
Copy link
Contributor Author

tvdijen commented May 13, 2020

Yes, the downside of changes like this it that it touches literally every method/file, but the benefits for fully typehinting a library like this are huge..
I can split it in multiple smaller PR's if that helps you to review things?

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.

3 participants