John Snow <js...@redhat.com> writes:
> On 2/3/21 9:23 AM, Markus Armbruster wrote:
>> John Snow <js...@redhat.com> writes:
>>
>>> This is only used to pass in a dictionary with a comment already set, so
>>> skip the runaround and just accept the comment.
>>>
>>> This works because _tree_to_qlit() treats 'if': None; 'comment': None
>>> exactly like absent 'if'; 'comment'.
>> Confusing, because the two paragraphs talk about two different
>> things:
>> 1. Actual arguments for @extra are either None or {'comment':
>> comment}.
>> Simplify: replace parameter @extra by parameter @comment.
>> 2. Dumb down the return value to always be of the form
>> (obj {'if': ifcond, 'comment': comment})
>>
>
> I think you are drawing attention to the fact that 'if' and 'comment'
> are now always present in this dict instead of conditionally present.
Correct.
> (else, I have misread you. (I think you are missing a comma.))
I am! I meant to write
(obj, {'if': ifcond, 'comment': comment})
>> I suspect splitting the patch is easier than crafting a clear commit
>> message for the combined one.
>>
>
> I wouldn't have considered to break out such a small change into two
> even smaller changes, but as you are in charge here ... Okey Dokey.
>
> (meta-tangent: [1])
[...]
> [1] As a matter of process, I sometimes find it cumbersome to
> intentionally engineer an intermediary state when I jumped straight
> from A->C in my actual editing.
Yes, the extra work can be cumbersome. But then writing a neat commit
message for a commit that does two things can also be cumbersome.
"Split and write two straightforward commit messages" has proven easier
for me many times.
> I will usually keep such intermediary forms when they come about
> naturally in the course of development, but rarely seek to add them
> artificially -- it feels like a major bummer to engineer, test, and
> scrutinize code that's only bound to be deleted immediately after.
> Sometimes, it feels like a waste of reviewer effort, too.
It depends. Sometimes "don't split and write a complicated commit
message" is easier.
Which way you get to "commit message(s) don't confuse Markus" in this
particular case is up to you :)
> It's been years and I still don't think I have any real intuitive
> sense for this, which is ...unfortunate.
It's been years, and my intuition still evolves.
