[PATCH 1/3] macdrv: Add wintab32 implementation.

Huw Davies huw at codeweavers.com
Thu Mar 24 02:43:24 CDT 2022


On Wed, Mar 23, 2022 at 09:46:45PM -0600, Elaine Lefler wrote:
> On Wed, Mar 23, 2022 at 3:42 AM Huw Davies <huw at codeweavers.com> wrote:
> > Hi Elaine,
> >
> > Thanks for the patches.  Unfortunately at the moment this patch is too
> > large to review properly.  Could you try breaking it down into several
> > smaller, self-contained patches?
> >
> > Huw.
> 
> Hi Huw,
> 
> I can try. Unfortunately these changes are very co-dependent, so it's
> difficult to break them up in any logical way.
> 
> If you're okay with reviewing patches that aren't meaningful on their
> own, I could split it into the following:
> 1) Move/rename wintab definitions to reduce code duplication (large
> patch, but it's just a trivial refactor)

Yes, this is fine and even encouraged if it reduces the real changes
in the rest of the patches.  Looking at these bits of your patch,
you could probably even split the refactoring into a few patches.

The key point to bear in mind is to keep things simple for the
reviewer, so small, bit-sized patches are strongly preferred.

> 2) Correct wintab32.dll behavior (smaller patch)

Again, ideal for a separate patch.

> 3) Add macdrv wintab implementation (still around 1000 lines, there's
> absolutely nothing I can do about this)

Note, it doesn't have to work straight away, so you can split it
in developmental increments.  One point though is that you
should try to avoid adding code that isn't called (dead code).

Huw.



More information about the wine-devel mailing list