Re: svn commit: r1884279 - in /subversion/trunk: build/ subversion/tests/cmdline/svntest/

futatuki_at_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
Received on 2020-12-12 04:35:24 CET