[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 Mar 2021 14:28:29 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> On 2/25/21 10:28 AM, Markus Armbruster wrote:
>> 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.
>>
>
> I guess it depends on where you wanted to see them; I thought it was
> strange that in check_foobar I couldn't see what foobar's valid keys
> were without scrolling back to the bottom of the file.
>
> Needing to see all the keys for the disparate forms together was not a
> case I ran into, but you can always drop this patch for now if you'd
> like.
Let's shelve it for now.
> I had some more adventurous patches that keeps pushing in this
> direction, but I don't know if it's really important.
When I work on a something, I tend to accumulate semi-related cleanups.
Including them is rarely a problem for reviewers when the result is two
dozen patches or so. When this isn't the case, I can:
* Pick them into a separate cleanup series to go before the real work.
Risks delaying the real work.
* Funnel them onto a cleanup branch to flushed later. Risks lonely
death in a rotting branch.
* Force myself to abstain from improving things that could really use
improvement. I call this "sitting on my hands".
This patch is in part three of at least six. Almost 90 patches up to
part three, with many more to come. I'm *desperate* to limit scope to
not get overwhelmed. Please consider the remedies above. This is a cry
for help, not a demand.
> My appetite in
> this area has waned since November.
I understand.