Oleh R. Nykyforchyn wrote:
Hello, I need an advice on what to do with some piece of code that I have written for about 3 years. I started to make changes in Wine keyboard driver because I was
If you would have cleaned it up and sent 3 years ago, then it would have been a part of the Wine for that long...
What you should do is: 1. Remove all the useless changes (white space, debugging code, extra hacks all over the place). 2. Pick an indentation style either use the code's style or 4 spaces for indentation (not 3, not 5, and no tabs please). 3. Separate independent changes from everything else (additional layouts, layout table, structures, header files, different debug channel(s), noop changes) and send them separately to wine-patches. 4. Clean up the logic changing code and brake it into separate independent functions or logical parts that can be applied one after another (detect layouts, init keyboard, helper functions, etc).
In fact I made cosmetic changes to 3 files in winex11.drv directory: x11drv.h, x11drv_main.c, event.c, but the fourth file keyboard.c was changed much more. About half of functions in it were rewritten, and it now easier to read new keyboard.c than diff output to understand the changes.
Don't do cosmetic changes. This makes it impossible to see what the real changes are. Please remove all this type of changes from your patch. This will account for about 80% of all the changes.
I got very reasonable advice from L.Rahyen to broke the patch into smaller parts. The problem is that I preserved the driver logic, but changed data structures, so any such patch (even very small one) will touch hundreds if
I didn't see all these hundreds of lines of code. I've seen 10-20 lines for each data structure change (not counting main_key_tab). Btw some of your structure changes are totally unnecessary and should be reverted (changes to event.c file).
lines across the whole file, probably introducing new bugs and being difficult to read and understand. Also, there is no reason to change code by a patch
Actually no. That would be the easiest thing to read - structure changes for each line. Nothing else.
Thanks for working on this for such a long time and for all your hard work! I'm sure there are lots of people who really like to see this sort of functionality in Wine.
Vitaliy.