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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fcntl: add module documentation #22191

Merged
merged 1 commit into from May 9, 2024
Merged

Conversation

mauke
Copy link
Contributor

@mauke mauke commented May 3, 2024

No description provided.

@jkeenan
Copy link
Contributor

jkeenan commented May 4, 2024

I'm glad that you took the time to write this documentation. However, it's such an extensive re-write that I question whether it should go into blead so soon before a production release.

@mauke
Copy link
Contributor Author

mauke commented May 4, 2024

Yes, it is a lot. But I'd argue this is only because the module barely had any documentation before and the little it had was partially inaccurate:

  • By default your system's F_* and O_* constants (eg, F_DUPFD and O_CREAT) and the FD_CLOEXEC constant are exported into your namespace.

    Not true; e.g. O_NOATIME or F_GETSIG are not exported by default.

  • See perlopentut to learn about the uses of the O_* constants with sysopen().

    That hasn't been the case since 2013; commit b25a8b1 removed all the sysopen bits from perlopentut.

  • In multiple places, "the S_I* constants" are mentioned, but some of the S_I* symbols aren't constants; they're one-argument functions (e.g. S_ISREG, S_ISDIR) and, deviating from the C macros, S_IFMT can be used both with and without an argument.

All in all, I think my patch is a massive improvement over the status quo.


That said, it is a pure documentation patch. None of the code is changed, so I don't see how this could break anything. Documentation updates are explicitly allowed under the "blead freeze".

What is the risk that you're seeing?

@mauke mauke merged commit 8cacb84 into Perl:blead May 9, 2024
29 checks passed
@mauke mauke deleted the fcntl-documentation branch May 9, 2024 05:11
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.

None yet

3 participants