Skip to content

Conversation

nicolas-grekas
Copy link
Contributor

@nicolas-grekas nicolas-grekas commented Sep 25, 2025

Without precluding the result of the vote at https://wiki.php.net/rfc/soft-deprecate-sleep-wakeup

I'm trying to help PHP 8.5 RMs get ready for RC2, as advised in this message:
https://externals.io/message/128643#128659

This PR reverts the commits on the topic, related to PRs #19682 and #19435
It also reverts #19827

Reviews welcome to ensure this can be merged on Oct. 4th if the vote passes.

/cc @DanielEScherzer @edorian

@nicolas-grekas
Copy link
Contributor Author

nicolas-grekas commented Sep 25, 2025

I'm wondering if it's worth reverting #19827 given that the affected classes already have __[un]serialize hooks since a few years now (since PHP 8.2).

@nicolas-grekas nicolas-grekas force-pushed the revert-sleep-wakeup-deprec branch from a7d54d1 to 0375921 Compare September 26, 2025 07:23
@nicolas-grekas
Copy link
Contributor Author

nicolas-grekas commented Sep 26, 2025

Thanks for the review, I removed the commits that reverted NEWS updates and also removed the revert of #19827, which can and should be kept to me.
I also added a commit to update the NEWS/UPGRADING files to mention the soft-deprecation.

@nicolas-grekas nicolas-grekas force-pushed the revert-sleep-wakeup-deprec branch from 0375921 to ea69ff6 Compare September 26, 2025 07:26
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Minor comments, but LGTM otherwise.

@nicolas-grekas nicolas-grekas force-pushed the revert-sleep-wakeup-deprec branch from 32ffdd0 to 0b844d4 Compare September 30, 2025 13:56
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Looks fine from a technical PoV

{
new Bar;
return [];
return [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the formatting changed here?

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.

6 participants