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

[BUGFIX] Avoid implicit nullable parameters #746

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Conversation

oliverklee
Copy link
Contributor

This avoids deprecation warnings in PHP 8.4.

Unfortunately, we need to remove the native type declaration for this to keep supporting PHP down to version 5.6.

This avoids deprecation warnings in PHP 8.4.

Unfortunately, we need to remove the native type declaration for
this to keep supporting PHP down to version 5.6.
@oliverklee
Copy link
Contributor Author

Demo with PHPUnit set to fail on deprecations: #745

Copy link
Contributor

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

Unfortunately, we need to remove the native type declaration for this to keep supporting PHP down to version 5.6.

I feared even worse: that it might not be possible to support all PHP versions, 5.6 through 8.4.

PHPDoc @params all present and correct where they needed to be reintroduced.

@JakeQZ JakeQZ merged commit 068853d into v8.x Oct 25, 2024
25 checks passed
@JakeQZ JakeQZ deleted the bugfix/84-deprecations branch October 25, 2024 22:45
@westonruter
Copy link
Contributor

For what it's worth, WordPress's minimum version requirement is now PHP 7.2. So if this was what was holding back abandoning PHP 5.6 support, then this is no longer the case.

@JakeQZ
Copy link
Contributor

JakeQZ commented Oct 31, 2024

For what it's worth, WordPress's minimum version requirement is now PHP 7.2. So if this was what was holding back abandoning PHP 5.6 support, then this is no longer the case.

Thanks. Yes, it may have been easier to make a 9.0.0 release supporting PHP 8.4, also ditching support for PHP<7,2, with other planned or already-implemented breaking changes then moved to a 10.0.0 milestone. WordPress wasn't a particular consideration; I think it was more about making a minor release to support PHP 8.4 if possible, so that other breaking changes already on main weren't included. We could have bumped the version to 9.0 on the release branch which doesn't include other potentially breaking changes, and merged the dropping of PHP<7.2 support to that. That may have been easier.

But by the time you've found out what the problem is, you've usually pretty-much found a fix, so there's then less work involved in carrying on that same path. In this case, that path leads to release that supports PHP versions from 5.6 through 8.4, though is missing some type annotations (but which are still present on the main branch for the next major release).

westonruter added a commit to westonruter/PHP-CSS-Parser that referenced this pull request Nov 4, 2024
…into fix/malformed-identifier

* 'v8.x' of https://github.com/MyIntervals/PHP-CSS-Parser: (46 commits)
  [CLEANUP] Autoformat the code (MyIntervals#749)
  [TASK] Prepare the 8.7.0 release (MyIntervals#742)
  [BUGFIX] Fix another implictly nullable parameter (MyIntervals#751)
  [CLEANUP] Autoformat the changelog (MyIntervals#750)
  [TASK] Configure PHPUnit to be more strict (MyIntervals#744)
  [BUGFIX] Avoid implicit nullable parameters (MyIntervals#746)
  [BUGFIX] Revert broken support for multiple comments (MyIntervals#741)
  [TASK] Add some more tests for parsing comments (MyIntervals#739)
  [TASK] Use fixed PHPUnit versions (MyIntervals#726)
  [TASK] Update PHPUnit (MyIntervals#725)
  [TASK] State since which version code is `@internal`/`@deprecated` (MyIntervals#723)
  [TASK] Deprecate the expansion of shorthand properties (MyIntervals#719)
  [TASK] Mark parsing-internal classes and methods as `@internal` (MyIntervals#711)
  [TASK] Deprecate `Parser::setCharset()` and `Parser::getCharset()` (MyIntervals#703)
  [FEATURE] Run the tests on CI with PHP 8.4 as well (MyIntervals#701)
  [TASK] Run the tests with PHP versions up to 8.3 (MyIntervals#697)
  [TASK] Also allow higher versions of PHPUnit (MyIntervals#682)
  [BUGFIX] Fix type errors in PHP strict mode (MyIntervals#695)
  [TASK] Block installations on unsupported higher PHP versions (MyIntervals#692)
  [BUGFIX] Avoid using `setUp()` in testcases (MyIntervals#683)
  ...
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 this pull request may close these issues.

3 participants