Skip to content

Support PHP 8.5#79

Merged
lippserd merged 3 commits intomainfrom
support-php-85
Feb 27, 2026
Merged

Support PHP 8.5#79
lippserd merged 3 commits intomainfrom
support-php-85

Conversation

@sukhwinder33445
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 commented Dec 2, 2025

PHP 8.4 changes:

Other changes:

  • Remove unused patches (the patched library is no longer in use).
  • Cleanup phpstan baseline.

resolves #78

@sukhwinder33445 sukhwinder33445 self-assigned this Dec 2, 2025
@cla-bot cla-bot bot added the cla/signed label Dec 2, 2025
@sukhwinder33445 sukhwinder33445 force-pushed the support-php-85 branch 3 times, most recently from dd7f1e8 to abdbabb Compare December 3, 2025 10:52
Copy link

@BastianLedererIcinga BastianLedererIcinga left a comment

Choose a reason for hiding this comment

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

The changes look good, everything should be fine once the issue with the dependencies is resolved.

@sukhwinder33445
Copy link
Contributor Author

The generation of some PDF fails, and the logs point to errors in the code related to wikimedia/less. I will test it again once Icinga/icingaweb2#5457 is ready.

@sukhwinder33445 sukhwinder33445 marked this pull request as draft January 7, 2026 16:31
@lippserd lippserd changed the title Support php 8.4/8.5 Support PHP 8.5 Jan 12, 2026
@sukhwinder33445
Copy link
Contributor Author

To make this branch ready for merging, the react/promise upgrade changes have been moved to a separate PR #84

@sukhwinder33445 sukhwinder33445 force-pushed the support-php-85 branch 2 times, most recently from 4c6f2db to 4244956 Compare January 13, 2026 16:41
@sukhwinder33445
Copy link
Contributor Author

karriere/pdf-merge:^3.4 does not support PHP 8.2 and 8.5 simultaneously, which leads to failing tests.

We could consider replacing it with sinanbekar/pdf-merger. However, currently PDFs cannot be generated because support for wikimedia/less.php version 5.4.0 is still missing. I will test it again as soon as it is ready.

@sukhwinder33445
Copy link
Contributor Author

I disabled the deprecation warning and tested sinanbekar/pdf-merger. Unfortunately, it is not compatible. I will try some other packages later.

@sukhwinder33445
Copy link
Contributor Author

Tested with the previously replaced lib iio/libmergepdf with applying this patch, and it works as expected. On PHP 8.4 and 8.5, it works when deprecation warnings are disabled.

@nilmerg if there is no important reason for the replacement, we could switch back to iio/libmergepdf.

@sukhwinder33445 sukhwinder33445 mentioned this pull request Jan 21, 2026
4 tasks
Since PHP 8.4 implicitly nullable parameter types are deprecated.
@sukhwinder33445 sukhwinder33445 marked this pull request as ready for review January 23, 2026 10:30
Copy link
Member

@lippserd lippserd left a comment

Choose a reason for hiding this comment

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

Please merge the two commits that are solely for cleaning up the dependency replacement made in #77. And please rephrase the commit message and description to clarify the intent:

Cleanup leftovers from dependency replacement

bc4d059 replaced dependencies but left outdated code, addressed here:

- Catch `Throwable` instead of removed `WebSocket\ConnectionException`
- Remove patch for replaced library

Please always use short commit hashes in commit descriptions instead of GitHub PR numbers, as these have no value outside of GitHub. And please do not use bullet points if there is only one point.

Please also adjust the PR description. The only changes required to support PHP 8.5 are now the fix for implicit nullable type declarations, right? I'm also not entirely sure if it makes sense to require the missing react/child-process issue, as it requires a PR in icinga-php-thirdparty that has not yet been merged. So why is the issue resolved in the first place? If this is really a strict requirement, please require the icinga-php-thirdparty PR or remove it.

bc4d059 replaced dependencies but left outdated code, addressed here:

- Catch `Throwable` instead of removed `WebSocket\ConnectionException`
- Remove patch for replaced library
@lippserd lippserd merged commit 067eecf into main Feb 27, 2026
13 checks passed
@lippserd lippserd deleted the support-php-85 branch February 27, 2026 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PHP 8.4/8.5 Support

3 participants