[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 15/16] qapi/expr.py: move related checks inside check_xxx
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 15/16] qapi/expr.py: move related checks inside check_xxx functions |
Date: |
Thu, 25 Feb 2021 16:28:02 +0100 |
John Snow <jsnow@redhat.com> writes:
> There's not a big obvious difference between the types of checks that
> happen in the main function versus the kind that happen in the
> functions. Now they're in one place for each of the main types.
>
> As part of the move, spell out the required and optional keywords so
> they're obvious at a glance. Use tuples instead of lists for immutable
> data, too.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
No objection to changing read-only lists to tuples (applies to previous
patch, too).
No objection to turning positional into keyword arguments where that
improves clarity.
I have doubts on the code motion. Yes, the checks for each type are now
together. On the other hand, the check_keys() are now separate. I can
no longer see all the keys at a glance.
- [PATCH v3 08/16] qapi/expr.py: add type hint annotations, (continued)
- [PATCH v3 13/16] qapi/expr.py: Modify check_keys to accept any Collection, John Snow, 2021/02/22
- [PATCH v3 16/16] qapi/expr.py: Use an expression checker dispatch table, John Snow, 2021/02/22
- [PATCH v3 14/16] qapi/expr.py: Use tuples instead of lists for static data, John Snow, 2021/02/22
- [PATCH v3 15/16] qapi/expr.py: move related checks inside check_xxx functions, John Snow, 2021/02/22
- Re: [PATCH v3 15/16] qapi/expr.py: move related checks inside check_xxx functions,
Markus Armbruster <=