Hi Marcel, On 06/28/2011 08:38 PM, Marcel Holtmann wrote: > Hi Denis, > >>>>> @@ -658,6 +703,11 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const >>>>> char *remote, >>>>> ipcp_set_server_info(ppp->ipcp, r, d1, d2); >>>>> } >>>>> >>>>> +void g_at_ppp_set_acfc_enabled(GAtPPP *ppp) >>>> >>>> I really do want the signature to be g_at_ppp_set_acfc_enabled(GAtPPP >>>> *ppp, gboolean enabled) >>>> >>>> There are cases where we might re-use the PPP object with different >>>> parameters. >>> >>> what is wrong with just g_at_ppp_set_acfc(GAtPPP *ppp, gboolean enabled) >>> as function name. Duplicating enabled in the function name and as >>> parameter seems to be bit odd. >> >> For APIs I generally prefer: >> >> _set_foo() when foo is a 'thing', e.g int/string/struct/etc >> >> and _set_foo_enabled() when foo is on/off as that intent is clearer when >> reading the code. > > fair enough. > > However we have not been 100% consistent then here. > > g_at_server_set_echo() > g_at_hdlc_set_no_carrier_detect() > > Both look at bit fishy with this API intention. Should we fix them as > well then while at it?
Yes, I think set_echo definitely should become set_echo_enabled set_no_carrier_detect_enabled would pretty long and I wouldn't mind having a better name for this one... Regards, -Denis _______________________________________________ ofono mailing list [email protected] http://lists.ofono.org/listinfo/ofono
