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

[Christian König <christian.koenig@xxxxxxx>]

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