odb.git: Send captured-but-discarded output to DEVNULL

[rwe]

This avoids buffering captured output that's always discarded. The parameter change from nocapture to stdout also makes it possible to direct that output to some other file/pipe.

[rwe]

The CI error is unrelated to these changes, I believe: it's the GPG changes:

E                   FileNotFoundError: [Errno 2] No such file or directory: 'S.gpg-agent.extra'

[krobelus]

Oh well. I've opened #104 which should at least fix the S.gpg-agent.extra bit.
To test that we'd probably need to run a large number of CI jobs, not sure if that's worth it.

[rwe]

@mystor Not sure if there's a way to re-run the GitHub actions, but I believe this PR is good to go otherwise.

[rwe]

This PR is still good to go and had no conflicts, but has been rebased as well.

[krobelus]

OK. I guess the added consistency is because now input-related parameters come before all output-related ones.

[krobelus]

can we call this File to be consisten with the typing module?

[rwe]

For context, this definition is directly inlined from subprocess.pyi as the type of that Popen argument: https://github.com/python/typeshed/blob/98afaa4c76d4279846f278eb5be8b99c391e6a99/stdlib/subprocess.pyi#L24

It's inlined here because it's not an exported type name, so I wasn't certain it would be importable in all supported versions. Alternatively it could also be done as:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from subprocess import _FILE

My only very minor concern with calling it File is increasing drift from Popen's type.

Preference between:

• Leave it as is
• Import it
• Change name to File

?

[krobelus]

LGTM

[krobelus]

Ok, then I think leaving it is fine (maybe add a note in the commit message). Importing would be ideal if possible (perhaps even with an alias).
Not sure if the if TYPE_CHECKING check actually makes that easier since the CI runs type checking on all versions.

It's inlined here because it's not an exported type name, so I wasn't certain it would be importable in all supported versions.

I don't know much about Python, I guess it's just about the leading underscore (which by convention means "private")?

[rwe]

It's inlined here because it's not an exported type name, so I wasn't certain it would be importable in all supported versions.

I don't know much about Python, I guess it's just about the leading underscore (which by convention means "private")?

The leading underscore is the conventional hint, but the practical hiccup was mainly that subprocess.pyi (which the type checker uses) includes the symbol, but subprocess.py itself does not; so it only exists during type-checking and would fail with an ImportError if imported unguarded at runtime.

But I think you're right. Since CI runs on all versions, importing with a if TYPE_CHECKING guard is probably better. That won't ever fail for users, which was my main concern. I'll push something along those lines up and see how it looks. Thank you for the reviews, by the way!

[rwe]

I believe the CI failure is again unrelated. See #113 for (potential) fix to the race conditions in the current editor-testing code.

[rwe]

If it’s possible to rerun those tests, please do!

[mystor]

Thanks for the fixes!