On 13.10.21 17:07, John Snow wrote:
On Wed, Oct 13, 2021 at 7:50 AM Hanna Reitz <hre...@redhat.com> wrote:
On 04.10.21 23:04, John Snow wrote:
> Now, 297 is just the iotests-specific incantations and
linters.py is as
> minimal as I can think to make it. The only remaining element in
here
> that ought to be configuration and not code is the list of skip
files,
Yeah...
> but they're still numerous enough that repeating them for mypy and
> pylint configurations both would be ... a hassle.
I agree.
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> tests/qemu-iotests/297 | 72 +++---------------------------
> tests/qemu-iotests/linters.py | 83
+++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+), 67 deletions(-)
> create mode 100644 tests/qemu-iotests/linters.py
I’d like to give an A-b because now the statuscode-returning
function is
in a library. But I already gave an A-b on the last patch precisely
because of the interface, and I shouldn’t be so grumpy.
Reviewed-by: Hanna Reitz <hre...@redhat.com>
I'm not entirely sure I understand your dislike(?) of status codes.
I'm not trying to ignore the feedback, but I don't think I understand
it fully.
It’s the fact that we only use status codes because they are part of the
interface of shell commands. A python function isn’t a shell command,
so I find it weird to use that interface there. Returning True/False
would make more sense, for example.
I understand we have the same thing with qemu* commands in iotests.py,
so I shouldn’t be so stuck on it...
Would it be better if I removed check=False and allowed the functions
to raise an Exception on non-zero subprocess return? (Possibly having
the function itself print the stdout on the error case before re-raising.)
Yes, I would like that better! :)
Hanna
