← Back to team overview

openerp-community-reviewer team mailing list archive

Re: [Merge] lp:~openerp-sage50/openerp-sage-50/6.1-exportsage50 into lp:openerp-sage-50

 

> please fold lines at approx 80 cols
> 
> There are some ' ' missing after end of sentence '.' (e.g line 213-214)
> 
> line 399: don't use a dictionary as default argument to a method. Use None and
> test for None-ness inside if needed
> line 405: I suggest using a StringIO object rather that using string
> concatenations.
> line 409: please remove this.
> line 410: pass context to browse()
> line 411: to be deleted
> 
> line 429: I suggest rewriting this as:
> fields = [customer_name, oneTimefield, contact_name, street1, street2,
>           city, province_state, zip_code, country, phone1, mobile, fax, email,
>          ]
> customer = ','.join(['"%s"' % field for fied in fields])
> 
> Or even better, use the csv module: you import it but don't use it. Why?
> 
> same thing for 445 and 470
> 
> This will probably generate a malformed file if one of the fields  has a
> double quote inside. I'll grant you this is unlikely, but you may want to
> sanitize before writing.
> 
> line 441 and 444, and similar lines in the code: use a string literal. Esp if
> you need "0.00" on line 444, since str(0.00) is "0.0"


Thanks for your comment
-- 
https://code.launchpad.net/~savoirfairelinux-openerp/openerp-sage-50/6.1-exportsage50/+merge/158246
Your team OpenERP Community Reviewer is requested to review the proposed merge of lp:~savoirfairelinux-openerp/openerp-sage-50/6.1-exportsage50 into lp:openerp-sage-50/6.1.