← Back to team overview

openerp-sage50 team mailing list archive

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

 

Review: Needs Fixing code review, no test

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"



-- 
https://code.launchpad.net/~openerp-sage50/openerp-sage-50/6.1-exportsage50/+merge/158246
Your team OpenERP Sage 50 is subscribed to branch lp:openerp-sage-50.


Follow ups

References