-
Notifications
You must be signed in to change notification settings - Fork 800
Add initial version of the PHP 8.2 migration guide #1799
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
Conversation
cb25bac
to
a6a8199
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much!
Partial review done; I'll try to review the rest ASAP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(more review to come :)
|
||
<para> | ||
<function>glob</function> returns empty array if all paths are restricted by open_basedir. | ||
Previously the error was returned but that behavior was not consistent and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error was returned? What does that mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bukka can you clarify?
As I just copy pasted the wording from UPGRADING
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously if all paths were restricted by open_basedir
, false
was returned and typical open basedir restriction warning was emitted. The consistency note means that there was no warning if just some paths were restricted.
<para> | ||
<function>dba_open</function> and <function>dba_popen</function> | ||
now have the following enforced signature: | ||
<methodsynopsis> | ||
<type class="union"><type>resource</type><type>false</type></type><methodname>dba_open</methodname> | ||
<methodparam><type>string</type><parameter>path</parameter></methodparam> | ||
<methodparam><type>string</type><parameter>mode</parameter></methodparam> | ||
<methodparam choice="opt"><type class="union"><type>string</type><type>null</type></type><parameter>handler</parameter><initializer>&null;</initializer></methodparam> | ||
<methodparam choice="opt"><type>int</type><parameter>permission</parameter><initializer>0644</initializer></methodparam> | ||
<methodparam choice="opt"><type>int</type><parameter>map_size</parameter><initializer>0</initializer></methodparam> | ||
<methodparam choice="opt"><type class="union"><type>int</type><type>null</type></type><parameter>flags</parameter><initializer>&null;</initializer></methodparam> | ||
</methodsynopsis> | ||
</para> | ||
<para> | ||
<function>dba_fetch</function>'s optional skip argument is now at the end | ||
in line with PHP userland semantics. Its signature is now: | ||
<methodsynopsis> | ||
<type class="union"><type>string</type><type>false</type></type><methodname>dba_fetch</methodname> | ||
<methodparam><type class="union"><type>string</type><type>array</type></type><parameter>key</parameter></methodparam> | ||
<methodparam><type>resource</type><parameter>handle</parameter></methodparam> | ||
<methodparam><type>int</type><parameter>skip</parameter></methodparam> | ||
</methodsynopsis> | ||
|
||
The overloaded signature: | ||
<methodsynopsis> | ||
<type class="union"><type>string</type><type>false</type></type><methodname>dba_fetch</methodname> | ||
<methodparam><type class="union"><type>string</type><type>array</type></type><parameter>key</parameter></methodparam> | ||
<methodparam><type>int</type><parameter>skip</parameter></methodparam> | ||
<methodparam><type>resource</type><parameter>handle</parameter></methodparam> | ||
</methodsynopsis> | ||
is still accepted, but it is recommended to use the new standard variant. | ||
</para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that we want to be that explicit in the migration guide. Maybe telling users about the signature changes, and then detailing on the respective pages is preferable to avoid duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to have it in the migration guide, but maybe that's because I haven't gotten round to writing the doc changes for DBA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, maybe put it in the migration guide, and later consider to move the details to the actual docs.
e396a5d
to
704bd15
Compare
e328f08
to
18c999e
Compare
I think it's now finally ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some minor issues could be addressed, but overall this looks good to me. Thank you very much!
<para> | ||
<methodname>SplFileObject::getChildren</methodname> now has a tentative | ||
return type of <type>null</type>, previously it was | ||
<type class="union"><type>RecursiveIterator</type><type>null</type></type>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<para> | ||
The creation of dynamic properties is deprecated, unless the class opts in by | ||
using the <code>#[\AllowDynamicProperties]</code> attribute. | ||
<classname>stdClass</classname> allows dynamic properties. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I guess we should either have a dedicated page for stdClass
, or not to use the <classname>
markup, but rather link to its documentation (whereever that is).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #1846 to track this issue.
<para> | ||
Usage of the <literal>QPrint</literal>, <literal>Base64</literal>, | ||
<literal>Uuencode</literal>, and <literal>HTML-ENTITIES</literal> | ||
'text encodings' is deprecated for all Mbstring functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All occurences of "MBString"/"Mbstring" should likely have the same case; I think the former is favorable.
Oh, and don't forget to add the migration guide to doc-base/manual.xml.in after merging. |
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Those are a leftover from copy pasting the migration guide
Missed the new flags param
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
Not all are there as some turn out to be undocumented
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
4d4dd30
to
c5b0601
Compare
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de> Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de> Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
Will need a doc-base change to be added to the live version.
This covers the UPGRADING file.
I think it is best to merge this ASAP and handle adding examples and links in other PRs as needed.