On Thu, Sep 7, 2017 at 10:30 AM, Aleksander Morgado <
[email protected]> wrote:

> On Thu, Sep 7, 2017 at 2:10 PM, Aleksander Morgado
> <[email protected]> wrote:
> >>>      if (result.allowed == MM_MODEM_MODE_NONE)
> >>> -        g_simple_async_result_set_error (simple,
> >>> -                                         MM_CORE_ERROR,
> >>> -                                         MM_CORE_ERROR_FAILED,
> >>> -                                         "Unknown mobile station
> class:
> >>> '%s'",
> >>> -                                         response);
> >>> +        g_task_return_new_error (task, MM_CORE_ERROR,
> >>> MM_CORE_ERROR_FAILED,
> >>
> >>
> >> not sure about the convention, should 'task' and 'MM_CORE_ERROR,
> >> MM_CORE_ERROR_FAILED' on separate line?  otherwise lgtm
> >>
> >
> > Not sure I follow a hard rule for this one, truth be told, especially
> > in the g_set_error() and friends. But looks like we are already doing
> > the split in most of other places, so let me send a v2 with that
> > change.
> >
>
> I've updated these already and ended up splitting the plugin port into
> separate commits, which is definitely much better to follow, see the
> commits in this branch here:
> https://github.com/aleksander0m/ModemManager/tree/aleksander/wavecom-gtask
>
> What do you think?
>

+1 on separate patches. lgtm


>
> --
> Aleksander
> https://aleksander.es
>
_______________________________________________
ModemManager-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel

Reply via email to