banking-addons-team team mailing list archive

[Alexandre Fayolle - camptocamp <alexandre.fayolle@xxxxxxxxxxxxxx>]

Review: Needs Information code review, no test

I notice no context is provided and therefore none is propagated. 


Regarding the test 'if (not country_code) or not country_id' : 

* would "if not (country_code and country_id)" be more readable? (not sure about this, and see 2nd point below)

* from the code higher in the function, we get there if a country_code was provided but the corresponding country was not found. My understanding of the execution flow leads me to think  that the first part of the test is to guard against a NameError because country_id is only defined if country_code was defined. In that case, I recommend setting country_id to False at the beginning of the function, which allows to rewrite the test as "if not country_id"



-- 
https://code.launchpad.net/~therp-nl/banking-addons/ba61-multicompany_safe_partner_search/+merge/146795
Your team Banking Addons Team is subscribed to branch lp:banking-addons.