futat...@apache.org wrote on Thu, 10 Dec 2020 14:45 +00:00:
> Ensure close file descriptors by using context manager in test suite.
Suggest instead:
test suite: Ensure file descriptors are closed in a timely manner, using
context managers.
I.e.:
- Use "$area: $description" format
- Fix ungrammatical use of "close"
(alternative: "Ensure timely closure of file descriptors")
And since I'm reviewing already, a few more nits:
> Generally, it is true that as file descriptors are closed when their
> underlying objects are deleted, we don't need close them explicitly.
> However if a Python language implementation that uses other strategy
> than reference count model, there is no warranty it will happen
"if a Python language implementation uses a strategy other than reference
counting"
> immediately after when those objects lose the last reference. So we
> use context manager to ensure close() is called immediately after
> those objects to be unnecessary. This helps us to run 'make check'
> with PyPy even smaler limit of number of open files, although we
s/even/under an even/
s/smaler/lower/ (the adjective modifies "limit", not "number")
Consider dropping the symbolic name (RLIMIT_NOFILE) into that sentence
somewhere.
> don't recommend use it because it is much slower than CPython for
s/use it/use of it|to use it/
s/it/${whatever the pronoun refers to}/
> this purpose.
No comments on the diff itself, but I only skimmed it.
Cheers,
Daniel
