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

Fix PHP 8.2 deprecations in tests #150

Merged
merged 4 commits into from
Mar 15, 2023

Conversation

Xerkus
Copy link
Member

@Xerkus Xerkus commented Mar 14, 2023

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes

Description

Remove Middleware tests and their dev/optional dependencies since dependencies are not installable on PHP 8 unless platform is ignored and this package no longer supports any of PHP 7 versions. This allows to drop platform ignore used for CI.

Declare properties in tests that were declared dynamically to fix PHP 8.2 deprecations.

Xerkus added 3 commits March 15, 2023 09:02
Signed-off-by: Aleksei Khudiakov <aleksey@xerkus.pro>
Optional dependencies for deprecated middleware functionality are not
installable on PHP 8 unless platform is ignored.

Drop optional dev dependencies and tests depending on them.

Signed-off-by: Aleksei Khudiakov <aleksey@xerkus.pro>
Signed-off-by: Aleksei Khudiakov <aleksey@xerkus.pro>
@Xerkus Xerkus added the Bug Something isn't working label Mar 14, 2023
@Xerkus Xerkus added this to the 3.6.1 milestone Mar 14, 2023
@Xerkus Xerkus requested a review from Ocramius March 14, 2023 23:43
@Xerkus Xerkus self-assigned this Mar 14, 2023
@Xerkus
Copy link
Member Author

Xerkus commented Mar 15, 2023

PHPUnit\Framework\Error\Deprecated: Use of "static" in callables is deprecated this is caused by webmozart/assert transient dependency, it was fixed in 1.11

Not sure if it should be added to dev dependencies to avoid the issue and satisfy CI

…lowest

Signed-off-by: Aleksei Khudiakov <aleksey@xerkus.pro>
@@ -1,135 +0,0 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

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

I think we should drop these in 4.x, not 3.x 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

They are dropped in 4.x.
hey are also not usable in PHP 8 and they prevent proper php version resolution.

They were requiring PHP below 8, but again I see that "merge" commit: ad9267a#diff-388c804d6485a66f01b14f8f443ca49bfe180c14bb340fac584ff613129f6363L23

Copy link
Member Author

Choose a reason for hiding this comment

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

Full reasons for dropping it:

  • This requires dependencies not installable on any supported PHP versions. Using it is already on own risk.
  • It interferes with dependency resolution because platform ignores are required. Renovate can not operate on this.
  • Optional stratigility version supported here was tested on up to PHP 7.2.
  • This was deprecated since laminas-mvc 3.2
  • This functionality is entirely replaced by laminas/laminas-mvc-middleware 1.0. Replacement does not use tested code and properly declares dependencies.
  • No changes will be accepted for either of the pieces tested by removed tests.

Copy link
Member

Choose a reason for hiding this comment

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

Very understandable, thanks!

@@ -1,545 +0,0 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

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

Should be dropped in 4.x rather than 3.x?

@Ocramius Ocramius merged commit f12e801 into laminas:3.6.x Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants