On Monday 15 August 2011 12:16:55 Michael Mc Donnell wrote:
Stefan thought a previous version of this looked ok, and I haven't had any negative feedback on this version either.
The 104 kb / 3000 line patch is rather big, you could probably split it up in test+patch like you did with the other patches to make it obvious that the bulk of this code is the tests. That'll not only help Alexandre, but also people who try to debug potential regressions caused by this patch(*)
To avoid lots of todo_wine's just send the implementation first and then the tests. That's not how test driven development works, but most people send patches in this order because it's easier.
(*) Ok, a patch adding a new function is unlikely to cause a regression, but still.
On Tue, Aug 16, 2011 at 9:14 PM, Stefan Dösinger stefandoesinger@gmx.at wrote:
On Monday 15 August 2011 12:16:55 Michael Mc Donnell wrote:
Stefan thought a previous version of this looked ok, and I haven't had any negative feedback on this version either.
The 104 kb / 3000 line patch is rather big, you could probably split it up in test+patch like you did with the other patches to make it obvious that the bulk of this code is the tests. That'll not only help Alexandre, but also people who try to debug potential regressions caused by this patch(*)
To avoid lots of todo_wine's just send the implementation first and then the tests. That's not how test driven development works, but most people send patches in this order because it's easier.
(*) Ok, a patch adding a new function is unlikely to cause a regression, but still.
Ok, I've splitted in two and sent them to wine-patches. Thanks for suggesting to send them in reverse order. That was a nice trick to avoid all those todo_wine :-)
-
Michael Mc Donnell -
Stefan Dösinger