Comment 2 for bug 1007922

Revision history for this message
Alexis de Lattre (alexis-via) wrote :

Thanks for your interest in the OpenERP-Asterisk connector and thank you for your contribution !

I have a number of remarks/questions about your patch :

- line 8 / strip (0) from phone numbers : good idea, I'll take it

- line 19 : I would love to have more info about this dial_postfix ; could you point me to a web page that has some info about it ? Is it specific to SCCP ? Is it specific to a particular Cisco phone model ? Why do you name this parameter dial_postfix and not dial_suffix ?

- line 37 / add SCCP : OK

- line 48 : I am very much against starting every word with a capital letter. I know that OpenERP guys start every word with a capital letter, but my school lessons have always tought me to put a capital letter only on the first word. But I would love to get some pointers to a rule that say we should put a capital letter on every word in a software.

- line 57 / Move 'Settings/Companies/Asterisk servers' to 'Settings/Configuration/Asterisk Servers/Asterisk Servers' : you are right, it's more coherent to put it there.

- line 83 : why do you want to rename it ? I think that "Open calling partner" is more meaning full that "Open Calls". In fact, "Open Calls" in plural make me think "Open Call history"... it's misleading for me. Could you tell me why you would like such a change ?

- line 91 / Move 'Sales/Address Book/Open calling partners' to 'Sales/Phone Calls/Open Calls' : the "asterisk_click2dial" module only depends on "base", and the move you propose would make it depend on "crm" too ! I don't want the asterisk_click2dial module to depend on "crm"... only the module "asterisk_click2dial_crm" should depend on "crm".

I would love to get your feedback on my remarks.