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

Unit tests should be executed with PHP 8.0 #740

Closed
driehle opened this issue Oct 21, 2021 · 1 comment · Fixed by #739
Closed

Unit tests should be executed with PHP 8.0 #740

driehle opened this issue Oct 21, 2021 · 1 comment · Fixed by #739
Assignees
Labels
Milestone

Comments

@driehle
Copy link
Member

driehle commented Oct 21, 2021

I think I might have found a suitable approach for getting unit tests running with PHP 8.0.

I'd like to discuss the changes proposed in #739 with you. First, as discussed on Slack, we are using symbols from laminas-(mvc-)console in src without required that package explicitly (it is in suggest, though). Therefore, at first, I have identified these placed, added an appropriate check on top of the file and use trigger_error to generate a runtime error in case someone tries using that classes without having the suggestion installed.

Next, I have deprecated the related symbols so that we can remove them with the next major version. To allow running the unit tests with PHP 8, I used --ignore-platform-req=php. I know that this is not the best choice, but I think we don't really have a chance to do otherwise. Besides laminas-(mvc-)console, there is laminas-cache as well, which still isn't fully PHP8-compatible (at lest withouth manually replacing legacy cache adapters, see docs).

Personally, I think we can go this way. I cannot think of a scenario where this will break someones application. We are continuing to support under PHP 7.3 & 7.4 what we have supported before and we are continuing not to support under PHP 8.0 what we have never supported before. What do you guys think?

cc @TomHAnderson @greg0ire

@driehle driehle added the Bug label Oct 21, 2021
@driehle driehle added this to the 4.2.0 milestone Oct 21, 2021
@driehle driehle self-assigned this Oct 21, 2021
@greg0ire
Copy link
Member

Hi! I think I have found a BC-break, but I'm unsure of its impact. Besides that, the PR looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants