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@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:
- 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.
- Correct wintab32.dll behavior (smaller patch)
Again, ideal for a separate patch.
- 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.