Skip to content

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Dec 13, 2022

Fixes #10051

As I can't test this locally, I've just YOLOed the implementation and hopefully CI will tell me if I did something wrong.

I'm wondering if it makes sense to backport to PHP 8.1 and 8.2 as this seems necessary and is really self-contained.
8.1 RMs: @ramsey @patrickallaert, 8.2 RMs: @adoy @saundefined

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! The patch is fine (also checked locally).

Regarding the branch to target: in my opinion, PHP-8.2 should be a no-brainer; if we commit soon, it will make it into php-8.2.1RC1. Targeting PHP-8.1 is up to debate, but I'm not against that either.

@adoy
Copy link
Member

adoy commented Dec 13, 2022

Looks good to me for 8.2
I did not packaged 8.2.1 yet. I will probably do it at the end of the day (Eastern time) so if you can merge it before.

Thanks :)

@Girgias
Copy link
Member Author

Girgias commented Dec 13, 2022

Merged it as 52a891a for PHP-8.2.

Should we still wait for 8.1 RMs before closing?

@cmb69
Copy link
Member

cmb69 commented Dec 15, 2022

Should we still wait for 8.1 RMs before closing?

Good question. We still could ship this with PHP 8.1.15, but finally that's an RM decision. @ramsey, @patrickallaert, what do you think?

@ramsey
Copy link
Member

ramsey commented Dec 15, 2022

Seems very self-contained, so I don't mind it being in PHP 8.1. @patrickallaert ?

@Girgias
Copy link
Member Author

Girgias commented Apr 6, 2023

Closing this, can always re-open if it should end up in 8.1

@jabcreations
Copy link

For those encountering the error that leads your searching here I would like to point out that if you have your loops to iterate email messages written correctly you will not encounter the error and thus the need for the imap_is_open() function. I'm still happy to have it though I'm much happier to have realized that I don't need it.

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.

IMAP: there's no way to check if a IMAP\Connection is still open

5 participants