Re: [Openerp-community-reviewer] [Merge] lp:~aristobulo/web-addons/web-addons into lp:web-addons

2014-05-12 Thread Holger Brunn (Therp)
Review: Approve code review -- https://code.launchpad.net/~aristobulo/web-addons/web-addons/+merge/217277 Your team Web-Addons Core Editors is subscribed to branch lp:web-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to :

Re: [Openerp-community-reviewer] [Merge] lp:~numerigraphe-team/purchase-wkfl/7.0-add-purchase_delivery_split_date into lp:purchase-wkfl

2014-05-12 Thread Lionel Sausin - Numérigraphe
Thanks Joël, I've updated the module as you suggested. -- https://code.launchpad.net/~numerigraphe-team/purchase-wkfl/7.0-add-purchase_delivery_split_date/+merge/211374 Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl. -- Mailing list:

[Openerp-community-reviewer] [Merge] lp:~camptocamp/department-mgmt/analytic_department_vre into lp:department-mgmt

2014-05-12 Thread Joël Grand-Guillaume
The proposal to merge lp:~camptocamp/department-mgmt/analytic_department_vre into lp:department-mgmt has been updated. Status: Needs review = Rejected For more details, see: https://code.launchpad.net/~camptocamp/department-mgmt/analytic_department_vre/+merge/218976 --

Re: [Openerp-community-reviewer] [Merge] lp:~aristobulo/web-addons/web_fields_masks into lp:web-addons

2014-05-12 Thread Holger Brunn (Therp)
Review: Needs Fixing You should put library files into /static/lib (why? If another module uses the same library, it would be loaded twice otherwise and is likely to clash) Further, #2492 means we can't have apostrophes or quotes in the input mask. Why don't you simply eval() this attribute?

Re: [Openerp-community-reviewer] [Merge] lp:~numerigraphe-team/ocb-addons/7.0-inventory-lines-sorted into lp:ocb-addons

2014-05-12 Thread Holger Brunn (Therp)
While the index is a good thing to have, I merge this already as we can add the index afterwards -- https://code.launchpad.net/~numerigraphe-team/ocb-addons/7.0-inventory-lines-sorted/+merge/210467 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list:

[Openerp-community-reviewer] [Merge] lp:~alhashash/ocb-addons/7.0-alhashash-bug1179705 into lp:ocb-addons

2014-05-12 Thread noreply
The proposal to merge lp:~alhashash/ocb-addons/7.0-alhashash-bug1179705 into lp:ocb-addons has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~alhashash/ocb-addons/7.0-alhashash-bug1179705/+merge/217681 --

[Openerp-community-reviewer] [Merge] lp:~numerigraphe-team/ocb-addons/7.0-inventory-lines-sorted into lp:ocb-addons

2014-05-12 Thread noreply
The proposal to merge lp:~numerigraphe-team/ocb-addons/7.0-inventory-lines-sorted into lp:ocb-addons has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~numerigraphe-team/ocb-addons/7.0-inventory-lines-sorted/+merge/210467 --

[Openerp-community-reviewer] [Merge] lp:~therp-nl/ocb-addons/6.1-lp1315367-task_work_timesheet_lines_False into lp:ocb-addons/6.1

2014-05-12 Thread noreply
The proposal to merge lp:~therp-nl/ocb-addons/6.1-lp1315367-task_work_timesheet_lines_False into lp:ocb-addons/6.1 has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~therp-nl/ocb-addons/6.1-lp1315367-task_work_timesheet_lines_False/+merge/218060

Re: [Openerp-community-reviewer] [Merge] lp:~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM into lp:ocb-addons

2014-05-12 Thread Holger Brunn (Therp)
Review: Approve code review -- https://code.launchpad.net/~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM/+merge/217049 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to :

Re: [Openerp-community-reviewer] [Merge] lp:~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM into lp:ocb-addons

2014-05-12 Thread Lionel Sausin - Numérigraphe
Review: Abstain co-author Thanks Holger, I've updated Loïc's patch according to your suggestion. Of course I'll abstain now that I'm co-author :) -- https://code.launchpad.net/~numerigraphe-team/ocb-addons/7.0-fill-inventory-OOM/+merge/217049 Your team OpenERP Community Backports is subscribed

Re: [Openerp-community-reviewer] [Merge] lp:~lmi/ocb-addons/7.0-opw-596916-rgo into lp:ocb-addons

2014-05-12 Thread Holger Brunn (Therp)
Review: Approve code review -- https://code.launchpad.net/~lmi/ocb-addons/7.0-opw-596916-rgo/+merge/218476 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to :

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/ocb-addons/ocb-7.0-fix_1302630_document_search_order_by-rde into lp:ocb-addons

2014-05-12 Thread Holger Brunn (Therp)
Review: Approve code review -- https://code.launchpad.net/~camptocamp/ocb-addons/ocb-7.0-fix_1302630_document_search_order_by-rde/+merge/214486 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post

[Openerp-community-reviewer] [Merge] lp:~lmi/ocb-addons/7.0-opw-596916-rgo into lp:ocb-addons

2014-05-12 Thread Holger Brunn (Therp)
The proposal to merge lp:~lmi/ocb-addons/7.0-opw-596916-rgo into lp:ocb-addons has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~lmi/ocb-addons/7.0-opw-596916-rgo/+merge/218476 --

Re: [Openerp-community-reviewer] [Merge] lp:~alhashash/ocb-addons/7.0-bug-1219204 into lp:ocb-addons

2014-05-12 Thread Holger Brunn (Therp)
Review: Needs Fixing #24 don't loose the context here. I propose context=dict(context or {}, regenerate_opening=True) Then you should move #9 before the loop, as we don't need to loop if this key is set -- https://code.launchpad.net/~alhashash/ocb-addons/7.0-bug-1219204/+merge/217676 Your team

Re: [Openerp-community-reviewer] [Merge] lp:~vauxoo/server-env-tools/7.0_merge_partner_duplicates into lp:server-env-tools

2014-05-12 Thread Holger Brunn (Therp)
What is the status if this MP in regard to https://code.launchpad.net/~camptocamp/partner-contact-management/add-base_partner_merge/+merge/189616 ? So where is the main development going on? -- https://code.launchpad.net/~vauxoo/server-env-tools/7.0_merge_partner_duplicates/+merge/170122 Your

Re: [Openerp-community-reviewer] [Merge] lp:~savoirfairelinux-openerp/lp-community-utils/pep394 into lp:lp-community-utils

2014-05-12 Thread Leonardo Pistone - camptocamp
Review: Approve If I understand correctly, we are trading a small smoothness advantage for old distributions vs. more up-to-date ones that received pep394. I lean a bit for calling python2 explicitly. I prefer those with old distributions to go through some (easy) hoops to comply with

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/department-mgmt/add-account-department-fix-default-anyltic-jge into lp:department-mgmt

2014-05-12 Thread Joël Grand-Guillaume
Hi, Thanks for the review I missed the trigger :( I corrected all the points. Regards, Joël -- https://code.launchpad.net/~camptocamp/department-mgmt/add-account-department-fix-default-anyltic-jge/+merge/218807 Your team Department Core Editors is subscribed to branch lp:department-mgmt. --

Re: [Openerp-community-reviewer] [Merge] lp:~savoirfairelinux-openerp/account-invoicing/account_invoice_line_merge into lp:account-invoicing

2014-05-12 Thread Holger Brunn (Therp)
Review: Needs Fixing I think the problem Joël means is that the sale_origin addon simply contains the code for purchase_origin. Then for #344ff: I think it's a bad idea to copy the original function and break inheritance. Better call super and set the origin field in the dictionary you get

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/department-mgmt/add-account-department-fix-default-anyltic-jge into lp:department-mgmt

2014-05-12 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review Thanks! LGTM -- https://code.launchpad.net/~camptocamp/department-mgmt/add-account-department-fix-default-anyltic-jge/+merge/218807 Your team Department Core Editors is subscribed to branch lp:department-mgmt. -- Mailing list:

Re: [Openerp-community-reviewer] [Merge] lp:~vauxoo/server-env-tools/7.0_merge_partner_duplicates into lp:server-env-tools

2014-05-12 Thread Yannick Vaucher @ Camptocamp
The developments were made in parallel. Both are backport of v8 merge wizard. Nhomar said he wanted to test it. But we had no feed back. https://code.launchpad.net/~camptocamp/partner-contact-management/add-base_partner_merge/+merge/189616/comments/436062 Nor I took the time to fully compare

Re: [Openerp-community-reviewer] [Merge] lp:~nicolariolini/account-invoicing/add_module_account_payment_term_month into lp:account-invoicing

2014-05-12 Thread Holger Brunn (Therp)
Review: Needs Fixing #197ff, #202ff missing spaces at end of strings @218ff can't you avoid code duplication and breaking inheritance by using super's result and simply adding the months as applicable? --

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/openerp-rma/7.0-crm_claim_rma_auto_set_warranty-rde into lp:openerp-rma

2014-05-12 Thread Romain Deheele - Camptocamp
Hi Benoit, I'm 100% ok with you, I changed it. Thanks for the suggestion. Romain -- https://code.launchpad.net/~camptocamp/openerp-rma/7.0-crm_claim_rma_auto_set_warranty-rde/+merge/218666 Your team OpenERP RMA is subscribed to branch lp:openerp-rma. -- Mailing list:

[Openerp-community-reviewer] [Merge] lp:~camptocamp/server-env-tools/7.0-monitoring into lp:server-env-tools

2014-05-12 Thread Alexandre Fayolle - camptocamp
The proposal to merge lp:~camptocamp/server-env-tools/7.0-monitoring into lp:server-env-tools has been updated. Status: Needs review = Work in progress For more details, see: https://code.launchpad.net/~camptocamp/server-env-tools/7.0-monitoring/+merge/215138 --

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/server-env-tools/7.0-monitoring into lp:server-env-tools

2014-05-12 Thread Alexandre Fayolle - camptocamp
thanks for the review, Daniel. I'm setting this back to WIP until I get the time for take your comments into account. -- https://code.launchpad.net/~camptocamp/server-env-tools/7.0-monitoring/+merge/215138 Your team Server Environment And Tools Core Editors is subscribed to branch

Re: [Openerp-community-reviewer] [Merge] lp:~agilebg/hr-timesheet/imp_hr_attendance_analysis_roundings into lp:hr-timesheet

2014-05-12 Thread Pedro Manuel Baeza
Review: Needs Fixing code review Hi, Lorenzo, Please use correct indentation in if sentence for PEP8 compliance (higher than nested block). Functionally speaking, it seems good, but maybe you can do: if (float_end_time - float_start_time) 0.001: and simplify the condition. Regards. --

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/sale-wkfl/sale-wkfl-sale-multi-journal into lp:sale-wkfl

2014-05-12 Thread Pedro Manuel Baeza
Review: Needs Fixing code review Hi, David, in that case, please put a journal_id field on sale.order, that is filled by default with journal_id selected on shop, but that can be changed and passed to invoice. On v8, when shops dissapear, you have to only remove sale.shop part. Regards. --

Re: [Openerp-community-reviewer] [Merge] lp:~alhashash/ocb-addons/7.0-bug-1317455-check-show-amount-in-words into lp:ocb-addons

2014-05-12 Thread Pedro Manuel Baeza
Review: Approve code review Thanks for the fix and for the detailed explanation of the bug. Regards. -- https://code.launchpad.net/~alhashash/ocb-addons/7.0-bug-1317455-check-show-amount-in-words/+merge/218930 Your team OpenERP Community Backports is subscribed to branch lp:ocb-addons. --

[Openerp-community-reviewer] [Bug 1318668] [NEW] [7.0] Product view slow with many stock_locations

2014-05-12 Thread Florent Delayen
Public bug reported: Hi ! With 13000 stock locations, it takes nearly 10 seconds to see the product list view, and another 10 seconds each time you click anywhere related to a product. We analysed that bug, and saw that the fields_view_get request returns our 13000 locations and our

Re: [Openerp-community-reviewer] [Merge] lp:~invitu/ocb-server/7.0 into lp:ocb-server

2014-05-12 Thread Alexandre Fayolle - camptocamp
the thing is, it is different to store a birthday (which should still be machine parseable) and a birthdate. When the field is called birthdate, I expect a date. -- https://code.launchpad.net/~invitu/ocb-server/7.0/+merge/169658 Your team OpenERP Community Backports is subscribed to branch

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/sale-wkfl/sale-wkfl-sale-multi-journal into lp:sale-wkfl

2014-05-12 Thread David BEAL
Hi Pedro, Thanks for this idea but in ecommerce for example, shop allows to store several config values, not only journal. It seems more interesting to develop a module for shop in v8 -- https://code.launchpad.net/~akretion-team/sale-wkfl/sale-wkfl-sale-multi-journal/+merge/215440 Your team

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/stock-logistic-flows/7.0-picking_dispatch_wave-according-defined-number-of-sales_rde into lp:stock-logistic-flows

2014-05-12 Thread Romain Deheele - Camptocamp
Thanks Yannick, I've reworked terms and translation. Romain -- https://code.launchpad.net/~camptocamp/stock-logistic-flows/7.0-picking_dispatch_wave-according-defined-number-of-sales_rde/+merge/214568 Your team Stock and Logistic Core Editors is subscribed to branch lp:stock-logistic-flows.

Re: [Openerp-community-reviewer] [Merge] lp:~akretion-team/sale-wkfl/sale-wkfl-sale-multi-journal into lp:sale-wkfl

2014-05-12 Thread Pedro Manuel Baeza
Yeah, and that plans are compatible with the proposal, because you can have a basic module with only journal at sale order level, and an additional one with the dependency to new shop module with this feature. Regards. --

Re: [Openerp-community-reviewer] [Merge] lp:~numerigraphe-team/purchase-wkfl/7.0-add-purchase_delivery_split_date into lp:purchase-wkfl

2014-05-12 Thread Lorenzo Battistini - Agile BG
Review: Approve code review -- https://code.launchpad.net/~numerigraphe-team/purchase-wkfl/7.0-add-purchase_delivery_split_date/+merge/211374 Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to

Re: [Openerp-community-reviewer] [Merge] lp:~camptocamp/account-invoicing/account-analytic-required-vre into lp:account-invoicing

2014-05-12 Thread Lorenzo Battistini - Agile BG
Review: Needs Fixing Thanks Vincent, some style fixes: account_invoice_analytic_required/__init__.py:3:2: W291 trailing whitespace account_invoice_analytic_required/__init__.py:19:75: W291 trailing whitespace account_invoice_analytic_required/__init__.py:23:14: E271 multiple spaces after

Re: [Openerp-community-reviewer] [Merge] lp:~savoirfairelinux-openerp/purchase-wkfl/pallet-delivery-missing-security-access into lp:purchase-wkfl

2014-05-12 Thread Lorenzo Battistini - Agile BG
Review: Needs Fixing line 35: missing description -- https://code.launchpad.net/~savoirfairelinux-openerp/purchase-wkfl/pallet-delivery-missing-security-access/+merge/218508 Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl. -- Mailing list:

Re: [Openerp-community-reviewer] [Merge] lp:~agilebg/account-consolidation/7.0-bug-1296740-elbati into lp:account-consolidation/7.0

2014-05-12 Thread Lorenzo Battistini - Agile BG
Dear Yannick, I don't think this MP would break compatibility with account_financial_report_webkit. I installed account_financial_report_webkit and account_parralel_currency and I can use them without problems. It just doesn't copy the centralized field to the parallel accounts. For that, a

Re: [Openerp-community-reviewer] [Merge] lp:~pedro.baeza/purchase-wkfl/7.0-purchase_discount into lp:purchase-wkfl

2014-05-12 Thread Lorenzo Battistini - Agile BG
Review: Needs Fixing Thanks Pedro, line 349: I think discount should not be -100 too line 399: amount_untaxed and following fields are present 2 times some PEP8: purchase_discount/__openerp__.py:21:1: O603 Manifest license key is missing purchase_discount/__openerp__.py:29:5: O600 Warning

[Openerp-community-reviewer] [Merge] lp:~luc-demeyer/openerp-reporting-engines/7.0-report_xls-tz_fix into lp:openerp-reporting-engines

2014-05-12 Thread Luc De Meyer (Noviat)
Luc De Meyer (Noviat) has proposed merging lp:~luc-demeyer/openerp-reporting-engines/7.0-report_xls-tz_fix into lp:openerp-reporting-engines. Requested reviews: OpenERP Community Reviewer/Maintainer (openerp-community-reviewer) For more details, see:

Re: [Openerp-community-reviewer] [Merge] lp:~pedro.baeza/account-financial-tools/7.0-account_chart_update-enhanced into lp:account-financial-tools

2014-05-12 Thread Pedro Manuel Baeza
Hi, Yannick, Thanks for taking the time to review such a huge change that I have made on the module. I know that there's still a lot to do to comply with all the conventions, but this is a beginning motivated by some requirements not met in it. I have fixed things you have point about missing

Re: [Openerp-community-reviewer] [Merge] lp:~pedro.baeza/account-financial-tools/7.0-account_chart_update-enhanced into lp:account-financial-tools

2014-05-12 Thread Pedro Manuel Baeza
s/required=True from fill/required=True from wizard -- https://code.launchpad.net/~pedro.baeza/account-financial-tools/7.0-account_chart_update-enhanced/+merge/212074 Your team OpenERP Community Reviewer/Maintainer is subscribed to branch lp:account-financial-tools. -- Mailing list:

Re: [Openerp-community-reviewer] [Merge] lp:~pedro.baeza/purchase-wkfl/7.0-purchase_discount into lp:purchase-wkfl

2014-05-12 Thread Pedro Manuel Baeza
Hi, Lorenzo, thanks for your review. I have corrected flake8 output. But about negative restriction in the percentage amount, I don't think it's a good idea, because you can apply a surcharge of whatever percentage you want: for example, -100% to double original price, -200% to triple it, and