<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 7, 2017 at 10:30 AM, Aleksander Morgado <span dir="ltr"><<a href="mailto:aleksander@aleksander.es" target="_blank">aleksander@aleksander.es</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Sep 7, 2017 at 2:10 PM, Aleksander Morgado<br>
<<a href="mailto:aleksander@aleksander.es">aleksander@aleksander.es</a>> wrote:<br>
>>>      if (result.allowed == MM_MODEM_MODE_NONE)<br>
>>> -        g_simple_async_result_set_<wbr>error (simple,<br>
>>> -                                         MM_CORE_ERROR,<br>
>>> -                                         MM_CORE_ERROR_FAILED,<br>
>>> -                                         "Unknown mobile station class:<br>
>>> '%s'",<br>
>>> -                                         response);<br>
>>> +        g_task_return_new_error (task, MM_CORE_ERROR,<br>
>>> MM_CORE_ERROR_FAILED,<br>
>><br>
>><br>
>> not sure about the convention, should 'task' and 'MM_CORE_ERROR,<br>
>> MM_CORE_ERROR_FAILED' on separate line?  otherwise lgtm<br>
>><br>
><br>
> Not sure I follow a hard rule for this one, truth be told, especially<br>
> in the g_set_error() and friends. But looks like we are already doing<br>
> the split in most of other places, so let me send a v2 with that<br>
> change.<br>
><br>
<br>
</span>I've updated these already and ended up splitting the plugin port into<br>
separate commits, which is definitely much better to follow, see the<br>
commits in this branch here:<br>
<a href="https://github.com/aleksander0m/ModemManager/tree/aleksander/wavecom-gtask" rel="noreferrer" target="_blank">https://github.com/<wbr>aleksander0m/ModemManager/<wbr>tree/aleksander/wavecom-gtask</a><br>
<br>
What do you think?<br></blockquote><div><br></div><div>+1 on separate patches. lgtm</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Aleksander<br>
<a href="https://aleksander.es" rel="noreferrer" target="_blank">https://aleksander.es</a><br>
</font></span></blockquote></div><br></div></div>