Re: [PATCH 05/13] wineplugplay.sys: Common function to check if a bus is enabled
On 17.08.2016 15:36, Aric Stewart wrote:
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> --- dlls/wineplugplay.sys/Makefile.in | 1 + dlls/wineplugplay.sys/bus_hid_common.c | 65 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 dlls/wineplugplay.sys/bus_hid_common.c
Is it intentional that this patch does not contain any include file changes? Not sure if Alexandre agrees, but I personally think it might be useful to merge patches 5-7. When added separately they are basically dead code, and its difficult to review them without a usage example. Regards, Sebastian
The individuality of these are mostly an artifact of my refactoring and reordering. It would be trivial to merge them is Alexandre agrees that is ideal. -aric Sent from my iPhone.
On Aug 20, 2016, at 5:54 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote:
On 17.08.2016 15:36, Aric Stewart wrote: Signed-off-by: Aric Stewart <aric(a)codeweavers.com> --- dlls/wineplugplay.sys/Makefile.in | 1 + dlls/wineplugplay.sys/bus_hid_common.c | 65 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 dlls/wineplugplay.sys/bus_hid_common.c
Is it intentional that this patch does not contain any include file changes? Not sure if Alexandre agrees, but I personally think it might be useful to merge patches 5-7. When added separately they are basically dead code, and its difficult to review them without a usage example.
Regards, Sebastian
Codeweavers <aric(a)codeweavers.com> writes:
The individuality of these are mostly an artifact of my refactoring and reordering. It would be trivial to merge them is Alexandre agrees that is ideal.
It's not possible to meaningfully review dead code, so yes, if you add a function it should be used in the same patch. If that makes the patch too big it should be split along different lines. -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Codeweavers -
Sebastian Lackner