Re: [PATCH 05/13] wineplugplay.sys: Common function to check if a bus is enabled

[Sebastian Lackner]

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

[PATCH 05/13] wineplugplay.sys: Common function to check if a bus is enabled

[Codeweavers]

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

[PATCH 05/13] wineplugplay.sys: Common function to check if a bus is enabled

[Alexandre Julliard]

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