Re: [Xen-devel] [XEN PATCH v2 05/12] xen/include: remove include of Config.mk

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.01.2020 19:34, Anthony PERARD wrote:
> On Wed, Jan 29, 2020 at 04:33:02PM +0100, Jan Beulich wrote:
>> On 29.01.2020 16:28, Jan Beulich wrote:
>>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>>> It isn't necessary to include Config.mk here because this Makefile is
>>>> only used by xen/Rules.mk which already includes Config.mk.
>>>
>>> And so is xen/test/livepatch/Makefile afaics from its parent dir
>>> Makefile. With this also adjusted (or it explained why I'm seeing
>>> things incorrectly) ...
>>>
>>>> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>>>
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> And now I've seen that patch 6 does just this. I think such
>> common theme changes are, unless patches are overly large
>> already, better put all in on patch. Anyway - the R-b then
>> is unconditional.
> 
> I don't like squashing unrelated changes together. I though both changes
> deserved there own explanation in this case. They don't touch the same
> subsystem, they don't have the same set of maintainers.

Yes, the issue was in part because I noticed too late that
there was a 2nd, similar patch (and hence I went and checked
whether you've caught all instances where removal would be
possible). I understand your concerns, yet I think these two
aren't unrelated. Under a title "remove unnecessary includes
of Config.mk" both would have fit. But don't get me wrong,
I'm fine with them remaining split. A post-commit-message
remark clarifying this doesn't cover all instances would
have helped review nevertheless.

>> Another question: The cover letter doesn't say anything about
>> some (or most) patches here being independent of one another,
>> and hence the option of them going in out of order. The one
>> here looks to be entirely standalone, for example.
> 
> It is extra work to figure out which patch could be applied out of
> order. I would have independent patch at the beginning of the series,
> but if there aren't, it is probably because I haven't though they were
> important enough to think about applying them independently. I might try
> to reorder some patches in later version of a series to have them
> applied earlier.
> 
> As for this series, I do think applying most patches in order is
> important, changing the order may lead to unexpected breakage. That
> might not be true, but I don't want to spend time on checking that.

Fair enough. With my committer hat on, I just typically try
to spot opportunities of committing pieces, to reduce the
overall volume of to-be-resent patches.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel