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

Remove personalization in section reference/filesystem #2726

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

adielcristo
Copy link
Member

No description provided.

@adielcristo adielcristo marked this pull request as draft August 31, 2023 10:58
adielcristo referenced this pull request in php/doc-pt_br Aug 31, 2023
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.

Please remove the whitespace changes that add new lines, especially those just after the <!-- Revision --> tag

Also, while I understand the logic of changing where a line is broken to a new line those provide no value and just introduce noise for translations.

Those can be done in a separate PR with a [skip-revcheck] commit to not burden translations for nothing.

Comment on lines 30 to 32
<function>basename</function> is locale aware, so for it to see the correct
basename with multibyte character paths, the matching locale must be set
using the <function>setlocale</function> function.
Copy link
Member

Choose a reason for hiding this comment

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

This change looks pointless.

Copy link
Member Author

@adielcristo adielcristo Sep 30, 2023

Choose a reason for hiding this comment

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

Reverted.

@adielcristo
Copy link
Member Author

Please remove the whitespace changes that add new lines, especially those just after the <!-- Revision --> tag

Also, while I understand the logic of changing where a line is broken to a new line those provide no value and just introduce noise for translations.

Those can be done in a separate PR with a [skip-revcheck] commit to not burden translations for nothing.

I'll revert the line changes. I didn't know they could/should be done with a [skip-revcheck] commit.

@adielcristo
Copy link
Member Author

I'll revert the line changes. I didn't know they could/should be done with a [skip-revcheck] commit.

Formatting changes reverted.

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 nits

@@ -37,7 +37,8 @@
<para>
Note that <parameter>permissions</parameter> is not automatically
assumed to be an octal value, so to ensure the expected operation,
you need to prefix <parameter>permissions</parameter> with a zero (0).
<parameter>permissions</parameter> needs to be prefixed with a zero
(<literal>0</literal>).
Strings such as "g+w" will not work properly.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add the literal tag below for "g+w" too?

Comment on lines +161 to 166
<para>This enables PHP to interoperate with Macintosh systems, but defaults
to <literal>Off</literal>, as there is a very small performance penalty
when detecting the <literal>EOL</literal> conventions for the first line,
and also because people using carriage-returns as item separators under
Unix systems would experience non-backwards-compatible behaviour.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>This enables PHP to interoperate with Macintosh systems, but defaults
to <literal>Off</literal>, as there is a very small performance penalty
when detecting the <literal>EOL</literal> conventions for the first line,
and also because people using carriage-returns as item separators under
Unix systems would experience non-backwards-compatible behaviour.
</para>
<para>
This enables PHP to interoperate with Macintosh systems, but defaults
to <literal>Off</literal>, as there is a very small performance penalty
when detecting the <literal>EOL</literal> conventions for the first line,
and also because people using carriage-returns as item separators under
Unix systems would experience non-backwards-compatible behaviour.
</para>

While at it

Comment on lines 8 to 17
<!-- {{{ preface -->
<preface xml:id="intro.filesystem">
&reftitle.intro;
&no.requirement;
<para>
No external libraries are needed to build this extension, but if you want
PHP to support LFS (large files) on Linux, then you need to have a recent
glibc and you need compile PHP with the following compiler flags:
<literal>-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64</literal>.
However, for PHP to support <acronym>LFS</acronym> on Linux, a recent glibc
is required, and PHP needs to be compiled with the following compiler flags:
<option role="configure">-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64</option>.
</para>
</preface>
Copy link
Member

Choose a reason for hiding this comment

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

So, this seems to be duplicated with the setup.xml file, as it doesn't seem to respect the usual structure.

Maybe it makes sense to merge the content of setup.xml into book.xml but that's something I need to think of...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants