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

Fixes for PHP 8.1 #1130

Merged
merged 3 commits into from Aug 16, 2021
Merged

Fixes for PHP 8.1 #1130

merged 3 commits into from Aug 16, 2021

Conversation

jrmajor
Copy link
Contributor

@jrmajor jrmajor commented Jun 26, 2021

  • Avoid passing null to non-nullable parameters of internal functions
  • Skip Serializable test on PHP <8.1
  • Add support for tentative return types

@GrahamCampbell
Copy link
Contributor

Can you target the 1.3 branch please, so Laravel can get these fixes.

@jrmajor
Copy link
Contributor Author

jrmajor commented Jun 27, 2021

@GrahamCampbell Sorry, I wasn't aware that Mockery doesn't follow semantic versioning. I've never changed target branch in PR, so I'm not sure how to do it correctly, will it work if I just change it via GitHub interface?

I see Laravel 8 requires ^1.4 and 6 requires ~1.3.3|^1.4.2, so both current and LTS versions should be able to get this fixes. Why is it needed to target 1.3?

@GrahamCampbell
Copy link
Contributor

Mockey does follow semantic versioning, but the 1.4.x line has cranked the min PHP version. Laravel needs the 1.3.x line to continue to be maintained for the rest of 2021 (and I'm happy to do any and all additional work to make that happen). :)

@jrmajor
Copy link
Contributor Author

jrmajor commented Jun 27, 2021

Mockey does follow semantic versioning, but the 1.4.x line has cranked the min PHP version.

OK, I see. The strange version constraint made me think so. Wouldn't ~1.3.3 behave the same as ^1.3.3?

And please let me know what I need to do to change the target branch. I'm also happy to do this work, but in this case I'm not sure how. BTW, failing Laravel's tests are the reason I've opened this PR :)

@GrahamCampbell
Copy link
Contributor

Wouldn't ~1.3.3 behave the same as ^1.3.3?

No. ^ is a semver operator. ~ pins the major and minor in your example.

@jrmajor jrmajor changed the base branch from master to 1.3 June 27, 2021 09:00
@jrmajor jrmajor changed the base branch from 1.3 to master June 27, 2021 09:03
@jrmajor jrmajor changed the base branch from master to 1.3 June 27, 2021 09:12
@jrmajor
Copy link
Contributor Author

jrmajor commented Jun 27, 2021

@GrahamCampbell Thanks for your anwsers :) I've changed the target to 1.3, but I can't run tests (Test directory "tests/PHP56" not found).

@GrahamCampbell
Copy link
Contributor

Looks like we need to move to GH actions. I'll create an issue and do it soon.

jrfnl added a commit to jrfnl/ReflectionDocBlock that referenced this pull request Aug 1, 2021
While PHP 8.1 has not been released yet, it is common to allow such a build to fail.

The tests currently fail due to PHP 8.1 incompatibilities in Mockery.
A PR for (most of) these is already open in the Mockery repo: mockery/mockery#1130

Once Mockery has been made compatible with PHP 8.1, the `continue-on-error` setting should be revisited after verifying that all tests pass.
jrfnl added a commit to jrfnl/ReflectionDocBlock that referenced this pull request Aug 6, 2021
While PHP 8.1 has not been released yet, it is common to allow such a build to fail.

The tests currently fail due to PHP 8.1 incompatibilities in Mockery.
A PR for (most of) these is already open in the Mockery repo: mockery/mockery#1130

Once Mockery has been made compatible with PHP 8.1, the `continue-on-error` setting should be revisited after verifying that all tests pass.
@davedevelopment davedevelopment merged commit 0a999ca into mockery:1.3 Aug 16, 2021
@davedevelopment
Copy link
Collaborator

Merged. I'm a bit short on time right now, summer holidays, kids etc, so I've not given it a proper good looking at, trusting @GrahamCampbell 👍

@jrmajor
Copy link
Contributor Author

jrmajor commented Aug 16, 2021

@davedevelopment Thank you!

Mockery\Reflector::getReturnType() is different in 1.3 and 1.4, so you may use version from b4da544 to resolve this conflict when merging 1.3 to master.

@GrahamCampbell
Copy link
Contributor

That change to 1.4.x should probably also be backported to 1.3.x.

@jrmajor
Copy link
Contributor Author

jrmajor commented Aug 16, 2021

@GrahamCampbell The commit I mentioned is the version for 1.4, which on your request I later backported to 1.3. It had conflicts in this method, so this version may come in handy when merging this PR to 1.4.

@jrmajor jrmajor deleted the tentative-types branch August 16, 2021 16:12
@jrmajor jrmajor restored the tentative-types branch August 16, 2021 16:13
@ghostwriter ghostwriter mentioned this pull request Apr 20, 2023
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.

None yet

3 participants