[Spice-devel] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev

[Christian König]

Am 12.07.23 um 15:38 schrieb Uwe Kleine-König:
> Hello Maxime,
>
> On Wed, Jul 12, 2023 at 02:52:38PM +0200, Maxime Ripard wrote:
>> On Wed, Jul 12, 2023 at 01:02:53PM +0200, Uwe Kleine-König wrote:
>>>> Background is that this makes merge conflicts easier to handle and detect.
>>> Really?
>> FWIW, I agree with Christian here.
>>
>>> Each file (apart from include/drm/drm_crtc.h) is only touched once. So
>>> unless I'm missing something you don't get less or easier conflicts by
>>> doing it all in a single patch. But you gain the freedom to drop a
>>> patch for one driver without having to drop the rest with it.
>> Not really, because the last patch removed the union anyway. So you have
>> to revert both the last patch, plus that driver one. And then you need
>> to add a TODO to remove that union eventually.
> Yes, with a single patch you have only one revert (but 194 files changed,
> 1264 insertions(+), 1296 deletions(-)) instead of two (one of them: 1
> file changed, 9 insertions(+), 1 deletion(-); the other maybe a bit
> bigger). (And maybe you get away with just reverting the last patch.)
>
> With a single patch the TODO after a revert is "redo it all again (and
> prepare for a different set of conflicts)" while with the split series
> it's only "fix that one driver that was forgotten/borked" + reapply that
> 10 line patch.

Yeah, but for a maintainer the size of the patches doesn't matter. 
That's only interesting if you need to manually review the patch, which 
you hopefully doesn't do in case of something auto-generated.

In other words if the patch is auto-generated re-applying it completely 
is less work than fixing things up individually.

>   As the one who gets that TODO, I prefer the latter.

Yeah, but your personal preferences are not a technical relevant 
argument to a maintainer.

At the end of the day Dave or Daniel need to decide, because they need 
to live with it.

Regards,
Christian.

>
> So in sum: If your metric is "small count of reverted commits", you're
> right. If however your metric is: Better get 95% of this series' change
> in than maybe 0%, the split series is the way to do it.
>
> With me having spend ~3h on this series' changes, it's maybe
> understandable that I did it the way I did.
>
> FTR: This series was created on top of v6.5-rc1. If you apply it to
> drm-misc-next you get a (trivial) conflict in patch #2. If I consider to
> be the responsible maintainer who applies this series, I like being able
> to just do git am --skip then.
>
> FTR#2: In drm-misc-next is a new driver
> (drivers/gpu/drm/loongson/lsdc_crtc.c) so skipping the last patch for
> now might indeed be a good idea.
>
>>> So I still like the split version better, but I'm open to a more
>>> verbose reasoning from your side.
>> You're doing only one thing here, really: you change the name of a
>> structure field. If it was shared between multiple maintainers, then
>> sure, splitting that up is easier for everyone, but this will go through
>> drm-misc, so I can't see the benefit it brings.
> I see your argument, but I think mine weights more.
>
> Best regards
> Uwe
>