Re: [Openerp-community-reviewer] [Merge] lp:~agilebg/stock-logistic-flows/adding_product_customer_code_picking into lp:stock-logistic-flows

2014-06-23 Thread Lorenzo Battistini - Agile BG
2014-06-22 23:10 GMT+02:00 Alex Comba - Agile BG alex.co...@agilebg.com: Lorenzo, please have a look at my last commit, now it should be ok. Thanks Alex, maybe we could handle even more particular cases by using the address_get method to retrieve the company? --

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries

2014-06-23 Thread Sébastien BEAU - http : //www . akretion . com
Review: Needs Fixing Hi You don't have to test if available_option.mandatory is True but you can only use if available_option.mandatory as it's a boolean the is True is useless. --

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries

2014-06-23 Thread David BEAL (ak)
@sebastien, fix done, thank you for review -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- Mailing list:

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries

2014-06-23 Thread Sébastien BEAU - http : //www . akretion . com
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch lp:carriers-deliveries. -- Mailing list:

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries

2014-06-23 Thread Pedro Manuel Baeza
Review: Approve code review Hi, David, thanks for the improvement. You have to change module version number and provide a proper migration script for those that have previously the module installed. Regards. --

Re: [Openerp-community-reviewer] [Merge] lp:~serpentcs/server-env-tools/base_module_record into lp:server-env-tools

2014-06-23 Thread Alexandre Fayolle - camptocamp
Review: Needs Fixing code review, no tests Never used it, code looks shaggy, and there are not tests. Are there known issues? In it's current state, this module is not on par with the standards of OCA and it needs care and love. If someone has use for this module and is volunteering to

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/partner-contact-management/7.0-partner-helper-dbl into lp:partner-contact-management

2014-06-23 Thread David BEAL (ak)
Hi Pedro module is renamed and ready for merge Thank you Regards -- https://code.launchpad.net/~akretion-team/partner-contact-management/7.0-partner-helper-dbl/+merge/221501 Your team Partner and Contact Core Editors is subscribed to branch lp:partner-contact-management. -- Mailing list:

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/partner-contact-management/7.0-partner-helper-dbl into lp:partner-contact-management

2014-06-23 Thread Pedro Manuel Baeza
Review: Approve code review Great, thanks. Regards. -- https://code.launchpad.net/~akretion-team/partner-contact-management/7.0-partner-helper-dbl/+merge/221501 Your team Partner and Contact Core Editors is subscribed to branch lp:partner-contact-management. -- Mailing list:

[Openerp-community-reviewer] [Merge] lp:~initos.com/sale-reports/7.0-fix_lang_for_draft into lp:sale-reports

2014-06-23 Thread Katja Matthes
Katja Matthes has proposed merging lp:~initos.com/sale-reports/7.0-fix_lang_for_draft into lp:sale-reports. Requested reviews: Sale Core Editors (sale-core-editors) For more details, see: https://code.launchpad.net/~initos.com/sale-reports/7.0-fix_lang_for_draft/+merge/224106 Steps to

[Openerp-community-reviewer] [Merge] lp:~initos.com/account-invoice-report/7.0-fix_lang_for_draft into lp:account-invoice-report

2014-06-23 Thread Katja Matthes
Katja Matthes has proposed merging lp:~initos.com/account-invoice-report/7.0-fix_lang_for_draft into lp:account-invoice-report. Requested reviews: Account Core Editors (account-core-editors) For more details, see:

Re: [Openerp-community-reviewer] [Merge] lp:~initos.com/account-invoice-report/7.0-fix_lang_for_draft into lp:account-invoice-report

2014-06-23 Thread Pedro Manuel Baeza
Review: Needs Fixing code review Hi, Katja, You're right that current code doesn't work, but this is because the line that made the work is incorrect: try : lang = self.browse(cr, uid, inv_id).partner_id.lang inv_id is an id, not a list of ids, so you don't need to put index 0 to access

Re: [Openerp-community-reviewer] [Merge] lp:~initos.com/sale-reports/7.0-fix_lang_for_draft into lp:sale-reports

2014-06-23 Thread Pedro Manuel Baeza
Review: Needs Fixing code review This is the same as the other MP you have made (https://code.launchpad.net/~initos.com/account-invoice-report/7.0-fix_lang_for_draft/+merge/224109), so please change it in the same way. Regards. --

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:carriers-deliveries

2014-06-23 Thread David BEAL (ak)
Migrate script works for me. I think it could merge Only miss Yannick review -- https://code.launchpad.net/~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl/+merge/224027 Your team Stock and Logistic Core Editors is subscribed to branch

Re: [Openerp-community-reviewer] [Merge] lp:~alejandrosantana/account-financial-tools/7.0-account-financial_tools--add--account_account_extended_search into lp:account-financial-tools

2014-06-23 Thread Alexandre Fayolle - camptocamp
setting the MP as work in progress. Please set it back to needs review when requested changes are made. -- https://code.launchpad.net/~alejandrosantana/account-financial-tools/7.0-account-financial_tools--add--account_account_extended_search/+merge/200092 Your team OpenERP Community

Re: [Openerp-community-reviewer] [Merge] lp:~gdgellatly/server-env-tools/base-synchro-7.0 into lp:server-env-tools

2014-06-23 Thread Alexandre Fayolle - camptocamp
Review: Needs Fixing code review, no tests If people are willing to maintain this, then I'm not opposed to merging in server-env-tools. 2 points, though, to gain my approval: * the various size constraints seem overzealous to me (esp. server URL and domain which are obviously too short for

Re: [Openerp-community-reviewer] [Merge] lp:~pedro.baeza/account-financial-report/6.1-balance-reporting into lp:account-financial-report/6.1

2014-06-23 Thread Alexandre Fayolle - camptocamp
Review: Needs Fixing I would really love to see this code exercised by a couple of yaml tests... -- https://code.launchpad.net/~pedro.baeza/account-financial-report/6.1-balance-reporting/+merge/201166 Your team Account Report Core Editors is subscribed to branch

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/partner-contact-management/base-location-geonames-import into lp:partner-contact-management

2014-06-23 Thread Lorenzo Battistini - Agile BG
Hello Alexis, many thanks for the module. What do you think about creating the res.country.state records if they don't exist, before mapping them in the 'states' dictionary? The current version is supposed to correctly work with states if you first create states data by modules like