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

Add ReflectionFunctionAbstract tentative function and run shepherd with php 8.1 #8216

Closed
wants to merge 1 commit into from
Closed

Add ReflectionFunctionAbstract tentative function and run shepherd with php 8.1 #8216

wants to merge 1 commit into from

Conversation

othercorey
Copy link
Contributor

@othercorey othercorey commented Jul 4, 2022 •
edited

Trying to allow #8166 to run which uses the tentative return types from PHP 8.1.

Shouldn't shepherd and nit tests run against the same version?

@othercorey
Add ReflectionFunctionAbstract::hasTentativeReturnType() and getTenta…
0bcbe7d 
…tiveReturnType().

Run shepherd against php 8.1 to match unit tests and include the tentative functions for use in unit tests.
@orklah
Copy link
Collaborator

orklah commented Jul 4, 2022

Ideally, CI should run against every supported php version but that would take way too long. So having at least a version for unit tests and Psalm analysis is better than nothing. Could you revert that part?

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Jul 4, 2022
@othercorey
Copy link
Contributor Author

Ideally, CI should run against every supported php version but that would take way too long. So having at least a version for unit tests and Psalm analysis is better than nothing. Could you revert that part?

Do you mean revert the change in shepherd.yml? I couldn't get psalm to pass in the other PR. I assume it is using php 8.0 function signatures when you run shepherd with php 8.0?

@orklah
Copy link
Collaborator

orklah commented Jul 4, 2022

Yeah. For the other PR, just suppress the issue and explain that the signature doesn't exists before 8.1, it should be enough as long as it's only used in tests

@othercorey
Copy link
Contributor Author

Ok, I'll close this and update the other PR then.

@othercorey othercorey closed this Jul 4, 2022
@othercorey othercorey deleted the shepherd-8.1 branch July 4, 2022 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@othercorey @orklah