Re: [Openerp-community] [Merge] lp:~therp-nl/server-env-tools/6.1-mass_editing-fix_dataloss into lp:~openerp-community/server-env-tools/6.1-mass_editing

2014-03-19 Thread Guewen Baconnier @ Camptocamp
Review: Approve -- https://code.launchpad.net/~therp-nl/server-env-tools/6.1-mass_editing-fix_dataloss/+merge/201321 Your team OpenERP Community is subscribed to branch lp:~openerp-community/server-env-tools/6.1-mass_editing. ___ Mailing list: https:

Re: [Openerp-community] [Merge] lp:~therp-nl/server-env-tools/6.1-mass_editing-fix_dataloss into lp:~openerp-community/server-env-tools/6.1-mass_editing

2014-01-31 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing vals.get(split_key, False)[0][2] When split_key is not in vals, we'll have a problem :-) -- https://code.launchpad.net/~therp-nl/server-env-tools/6.1-mass_editing-fix_dataloss/+merge/201321 Your team OpenERP Community is subscribed to branch lp:~openerp-community/serve

[Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/7.0-product-customer-code-extraction into lp:openerp-product-attributes

2014-01-31 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~akretion-team/openerp-product-attributes/7.0-product-customer-code-extraction into lp:openerp-product-attributes has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~akretion-team/openerp-product-attributes/7

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/7.0-product-customer-code-extraction into lp:openerp-product-attributes

2014-01-31 Thread Guewen Baconnier @ Camptocamp
I set this MP as Work in progress as it is in "Needs Fixing" since weeks. Please set it back to "Need Review" once you think you are done. -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/7.0-product-customer-code-extraction/+merge/198296 Your team OpenERP Community is sub

Re: [Openerp-community] [Merge] lp:~agilebg/openerp-product-attributes/adding_pricelist_configurator_by_bom_7 into lp:openerp-product-attributes

2014-01-24 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information Seems great, but why is a BoM mandatory? One may want to generate such a pricelist over normal products, or do I miss something? -- https://code.launchpad.net/~agilebg/openerp-product-attributes/adding_pricelist_configurator_by_bom_7/+merge/203026 Your team OpenERP Comm

[Openerp-community] [Bug 1199386] Re: Email address regular expression needs work

2014-01-13 Thread Guewen Baconnier @ Camptocamp
This is a must read when dealing with regular expressions for emails: http://davidcel.is/blog/2012/09/06/stop-validating-email-addresses-with- regex/ -- You received this bug notification because you are a member of OpenERP Community, which is subscribed to the bug report. https://bugs.launchpad.

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-adding-stock_production_lot_custom_attributes-lep into lp:openerp-product-attributes

2014-01-10 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review Great module! LGTM -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/7.0-adding-stock_production_lot_custom_attributes-lep/+merge/195950 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes.

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-fix-1259975-migration-lep into lp:openerp-product-attributes

2013-12-19 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review LGTM Thanks -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/7.0-fix-1259975-migration-lep/+merge/198592 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Mailing lis

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/6.1-search-all-prodlot-attributes-lep into lp:openerp-product-attributes/6.1

2013-12-19 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review Great! thanks. l.102 I think that itertools.chain.from_iterable(expand(arg) for arg in args) is better (does not create intermediate lists) -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/6.1-search-all-prodlot-attributes-lep/+merge/199335

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-attribute-set-create-1256023 into lp:openerp-product-attributes

2013-11-28 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-attribute-set-create-1256023 into lp:openerp-product-attributes. Commit message: [FIX] prevent KeyError if no 'attribute_ids' is present in 'vals' Requested reviews: Product C

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-allow-to-bind-on-existing-field into lp:openerp-product-attributes

2013-11-26 Thread Guewen Baconnier @ Camptocamp
> Thanks Guewen. > > I suggest to copy/paste some or all of the explanation you give on the MP to a > comment in the code. > > Otherwise LGTM. > > L Makes sense! I added some explanations. Thanks for your review -- https://code.launchpad.net/~openerp-community/openerp-product-attributes/7.0-p

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-rights-attribute_set-1254609 into lp:openerp-product-attributes

2013-11-24 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-rights-attribute_set-1254609 into lp:openerp-product-attributes. Commit message: [ADD] access rights on attribute.set Requested reviews: Product Core Editors (product-core-editors) Related bugs

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-rights-attribute_set-1254609 into lp:openerp-product-attributes

2013-11-24 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~camptocamp/openerp-product-attributes/7.0-rights-attribute_set-1254609 into lp:openerp-product-attributes has been updated. Commit Message changed to: [ADD] access rights on attribute.set For more details, see: https://code.launchpad.net/~camptocamp/openerp-product-at

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name into lp:openerp-product-attributes

2013-11-13 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name/+merge/194998 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/7.0-french-translation-product_custom_attributes into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Approve Thanks LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/7.0-french-translation-product_custom_attributes/+merge/191356 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ M

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/7.0-french-translation-base_custom_attributes into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review thanks LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/7.0-french-translation-base_custom_attributes/+merge/191357 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes.

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/7.0-base_custom_attributes-translated-name into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Approve > I have one doubt before doing the merge: the change to translatable is handled > correctly by the core to keep previous data? > > Regards. Never saw problems with that. LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/7.0-base_custom_attributes-tra

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Resubmit The merge gives a conflict. Can you rebase on the latest revision please? -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name/+merge/188014 Your team OpenERP Community is subscribed to branch lp:openerp-pr

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-product-variant/fix-pep8-and-relative-import into lp:~anybox/openerp-product-variant/7.0

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information The MP's target is an Anybox's branch. Is it normal? -- https://code.launchpad.net/~openerp-community/openerp-product-variant/fix-pep8-and-relative-import/+merge/186351 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-product-variant/fix

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/port-add-product_multi_company_7.0-bis-jge into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review Made some changes, approve. -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/port-add-product_multi_company_7.0-bis-jge/+merge/192872 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. __

Re: [Openerp-community] [Merge] lp:~savoirfairelinux-openerp/openerp-product-attributes/product_dependencies into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing code review Hi Joao, In the on_change methods (l.124,135,144), I propose to add the "context=None" argument and propagate it to the calls (l.138,147). Thus, if someone want to use the context in a module extending yours, he will be able just be inheriting the views. Apart

Re: [Openerp-community] [Merge] lp:~agilebg/openerp-product-attributes/adding_product_category_code_7 into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Abstain > I'm agree that this is a very specific question that doesn't need to be merged > to the complete module, but it is also not very practical IMHO if it doesn't > add so much functionality. It will worth to have it if the module add also an > agrupation by this category code in main

Re: [Openerp-community] [Merge] lp:~sebastien.beau/openerp-product-attributes/openerp-product-attributes-product-dimension into lp:openerp-product-attributes

2013-11-11 Thread Guewen Baconnier @ Camptocamp
> Hi sorry for my late answer. > @Marc, I didn't know about this existing module > http://bazaar.launchpad.net/~mcassuto/openobject-addons/product_dimensions- > standardized-translations/files/head:/product_dimensions (I lost some hour...) > > @Maxime regarding the product_size, I agree with Humbe

Re: [Openerp-community] [Merge] lp:~openerp-community/purchase-wkfl/6.1-purchase_internal_validation into lp:purchase-wkfl/6.1

2013-11-11 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing code review > Thanks Guewen for the review. > > The message is meant to be customized. If you have a way to set it right, > please share it. Ok. I don't think we can get it right (on 7.0 we can set web.base.url in ir_config_parameter, I don't think that already existed in 6

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-base_custom_attributes-wiz-imp-yvr into lp:openerp-product-attributes

2013-10-29 Thread Guewen Baconnier @ Camptocamp
> My mice does not worth do be nice with it, but my wrist thanks you. s/mice/mouse/ -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/7.0-base_custom_attributes-wiz-imp-yvr/+merge/193068 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. __

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-base_custom_attributes-wiz-imp-yvr into lp:openerp-product-attributes

2013-10-29 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review My mice does not worth do be nice with it, but my wrist thanks you. -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/7.0-base_custom_attributes-wiz-imp-yvr/+merge/193068 Your team OpenERP Community is subscribed to branch lp:openerp-product-attribu

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-fix-1245875-yvr into lp:openerp-product-attributes

2013-10-29 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review Fix seems correct to me, the selection needs to have every available models (I used the exact same function recently). LGTM -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/7.0-fix-1245875-yvr/+merge/193066 Your team OpenERP Community is subscribe

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-required-on-attributes-gbr into lp:openerp-product-attributes

2013-10-09 Thread Guewen Baconnier @ Camptocamp
> Hmm I fear that my MP is not applicable => if we put a required on an > attribute, it will add a NOT NULL on the column, this is a nonsense when using > attributes which can be different per products. > > However, we could add a field 'required_on_views' or something like that, > which does not

[Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-allow-to-bind-on-existing-field into lp:openerp-product-attributes

2013-10-09 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-allow-to-bind-on-existing-field into lp:openerp-product-attributes. Commit message: [IMP] allow to create an attribute on an existing (and not manual) field

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-required-on-attributes-gbr into lp:openerp-product-attributes

2013-10-09 Thread Guewen Baconnier @ Camptocamp
Hmm I fear that my MP is not applicable => if we put a required on an attribute, it will add a NOT NULL on the column, this is a nonsense when using attributes which can be different per products. However, we could add a field 'required_on_views' or something like that, which does not put the "

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name into lp:openerp-product-attributes

2013-10-07 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name/+merge/188014 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name into lp:openerp-product-attributes

2013-10-07 Thread Guewen Baconnier @ Camptocamp
You apply lower() on the string just before the filter, so anyway you can't have any uppercase chars, that's why I put \W. However, your regexp is more explicit. My remark was more on the usage of filter() which is inefficient vs re.sub(). Thanks for the changes. Approve (I don't remember how to

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name into lp:openerp-product-attributes

2013-10-02 Thread Guewen Baconnier @ Camptocamp
Thanks for the change! Why not re.sub(r'\W', '', string) Instead of filter((lambda x: re.search('[0-9a-z_]', x)), string) ? -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name/+merge/188014 Your team OpenERP Community

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes_limit_database_column_name into lp:openerp-product-attributes

2013-10-02 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing code review Hi, The next time, can you avoid to mix a cleaning and a fix? Because we can't know where is your change and what it does without parsing and searching through all the diffs. Can I propose a better name for the function "set_column_name"? "safe_column_name" may

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-no-attribute_id-on-option-creation-gbr into lp:openerp-product-attributes

2013-10-01 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-no-attribute_id-on-option-creation-gbr into lp:openerp-product-attributes. Commit message: [IMP] when creating a new option on a 'selection' attribute, the attribute_id should be pre-f

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-required-on-attributes-gbr into lp:openerp-product-attributes

2013-10-01 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-required-on-attributes-gbr into lp:openerp-product-attributes. Commit message: [ADD] base_custom_attributes: required attribute is not honored when defined on the ir.model.fields and is not

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-literal_eval-domain into lp:openerp-product-attributes

2013-10-01 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-literal_eval-domain into lp:openerp-product-attributes. Commit message: [FIX] an empty domain '[]' is eval'ed as True and is always used Requested reviews: Product Core Editor

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-literal_eval-domain into lp:openerp-product-attributes

2013-10-01 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~camptocamp/openerp-product-attributes/7.0-literal_eval-domain into lp:openerp-product-attributes has been updated. Commit Message changed to: [FIX] an empty domain '[]' is eval'ed as True and is always used For more details, see: https://code.launchpad.net/~camptocamp

[Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-attribute_set_id_on_template-gbr into lp:openerp-product-attributes

2013-09-30 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-attribute_set_id_on_template-gbr into lp:openerp-product-attributes has been updated. Commit Message changed to: [FIX] the relation to the attribute set should be on the product.template, not t

[Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-attribute_set_id_on_template-gbr into lp:openerp-product-attributes

2013-09-30 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~openerp-community/openerp-product-attributes/7.0-product_custom_attributes-attribute_set_id_on_template-gbr into lp:openerp-product-attributes. Requested reviews: Product Core Editors (product-core-editors) For more details, see: https

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/fix-product-attribute-group into lp:openerp-product-attributes

2013-09-30 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/fix-product-attribute-group/+merge/187814 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Mailing list: https:/

[Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/fix-product-attribute-group into lp:openerp-product-attributes

2013-09-30 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~akretion-team/openerp-product-attributes/fix-product-attribute-group into lp:openerp-product-attributes has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~akretion-team/openerp-product-attributes/fix-product-attrib

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-doc/7.0-community-review into lp:~openerp-community/openobject-doc/v7.0

2013-09-26 Thread Guewen Baconnier @ Camptocamp
Review: Approve Thanks for having took care of that. Seems good to me. -- https://code.launchpad.net/~openerp-community/openobject-doc/7.0-community-review/+merge/186295 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-doc/7.0-community-review. _

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-doc/7.0-community-review into lp:~openerp-community/openobject-doc/v7.0

2013-09-23 Thread Guewen Baconnier @ Camptocamp
Thanks for the changes. In the "Merging" section, I think it could be useful to remind that the commit message defined (if defined) on the MP should be used. It can seems obvious, but I'm not sure that everyone is aware of that. Conversely, ideally the person who propose a merge should also pro

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-doc/7.0-community-review into lp:~openerp-community/openobject-doc/v7.0

2013-09-18 Thread Guewen Baconnier @ Camptocamp
On 09/18/2013 04:04 PM, Stefan Rijnhart (Therp) wrote: > @Guewen: I still think the 3 working days is not very meaningful in a world > wide community, so I would prefer to go the other way and drop this > terminology in favour of 5 calendar days. Good point. Anyway we should drop one or the othe

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-doc/7.0-community-review into lp:~openerp-community/openobject-doc/v7.0

2013-09-18 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing Some comments if you have ideas how to improve them: -- I find this sentence a bit obscure: "Avoid resubmitting a MP if not explicitly intended. The MP will lose history of commit and that make more work for reviewers." I would prefer but still I'm not sure: "Avoid to use

Re: [Openerp-community] [Merge] lp:~openerp-community/purchase-wkfl/6.1-purchase_internal_validation into lp:purchase-wkfl/6.1

2013-08-30 Thread Guewen Baconnier @ Camptocamp
> Is not normal to have the localhost url in the messages? > > > +http://localhost:8080; You should read "Is it normal to have the localhost url in the messages?" -- https://code.launchpad.net/~openerp-community/purchase-wkfl/6.1-purchase_internal_validat

Re: [Openerp-community] [Merge] lp:~openerp-community/purchase-wkfl/6.1-purchase_internal_validation into lp:purchase-wkfl/6.1

2013-08-30 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information Is not normal to have the localhost url in the messages? +http://localhost:8080; -- https://code.launchpad.net/~openerp-community/purchase-wkfl/6.1-purchase_internal_validation/+merge/170305 Your team OpenERP Community is subsc

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/base_custom_attributes-inherited-domain into lp:openerp-product-attributes

2013-08-12 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test Great, LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/base_custom_attributes-inherited-domain/+merge/177243 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. _

Re: [Openerp-community] [Merge] lp:~mathieu-julius/openerp-product-attributes/openerp-product-attributes-add-rights-and-menu into lp:openerp-product-attributes

2013-08-11 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no tset LGTM -- https://code.launchpad.net/~mathieu-julius/openerp-product-attributes/openerp-product-attributes-add-rights-and-menu/+merge/176402 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes.

[Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/fix-create-attribute-group into lp:openerp-product-attributes

2013-08-07 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~akretion-team/openerp-product-attributes/fix-create-attribute-group into lp:openerp-product-attributes has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~akretion-team/openerp-product-attributes/fix-create-attribut

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/fix-create-attribute-group into lp:openerp-product-attributes

2013-08-07 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/fix-create-attribute-group/+merge/178991 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Mailing l

Re: [Openerp-community] [Merge] lp:~sebastien.beau/openerp-product-attributes/openerp-product-attributes-product-dimension into lp:openerp-product-attributes

2013-07-22 Thread Guewen Baconnier @ Camptocamp
> Hi Guewen, Sébastien, > > I wonder if we should not reuse existing code : > https://code.launchpad.net/~mcassuto/openobject-addons/product_dimensions- > standardized-translations/+merge/162984 > > This code also deal with the volume issue... > I agree, we should reuse the existing modules whe

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attrs-image-char-fix into lp:openerp-product-attributes

2013-07-11 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information I don't think that openerp.tools.ustr() and unicode().encode() serve the same purpose. openerp.tools.ustr() takes a byte string and try to convert it to unicode (with an optional default encoding or try to guess). It uses the unicode() constructor. While unicode().en

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/avoid-to-raise-when-no-custom-attributes into lp:openerp-product-attributes

2013-07-10 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/avoid-to-raise-when-no-custom-attributes/+merge/173897 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___

Re: [Openerp-community] [Merge] lp:~mathieu-julius/openerp-product-attributes/openerp-product-attributes-categ-view-inherit into lp:openerp-product-attributes

2013-07-10 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~mathieu-julius/openerp-product-attributes/openerp-product-attributes-categ-view-inherit/+merge/173902 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. _

Re: [Openerp-community] [Merge] lp:~sebastien.beau/openerp-product-attributes/openerp-product-attributes-product-dimension into lp:openerp-product-attributes

2013-06-24 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information Hi Sébastien, My impression is that the dimensions should be centimeters. At least we should have a configurable precision. I think you can remove this TODO: #TODO add a function field for volume? an onchange? If someone need a 'volume' field, he will add it, woul

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/mixin-extraction into lp:openerp-product-attributes

2013-06-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test Thanks for the change LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/mixin-extraction/+merge/151333 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes.

Re: [Openerp-community] [Merge] lp:~enlightx/openerp-product-attributes/7.0-bug-1190658-unidecode-dep into lp:openerp-product-attributes

2013-06-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~enlightx/openerp-product-attributes/7.0-bug-1190658-unidecode-dep/+merge/169248 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Mailing lis

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes-fix-view into lp:openerp-product-attributes

2013-06-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/openerp-product-attributes-fix-view/+merge/168461 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___

Re: [Openerp-community] [Merge] lp:~eoc/server-env-tools/6.1-mass_editing-fix_bug_1187937 into lp:~openerp-community/server-env-tools/6.1-mass_editing

2013-06-18 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information Is it really intended to be merge in lp:~openerp-community/server-env-tools/6.1-mass_editing and not in lp:server-env-tools/6.1 ? -- https://code.launchpad.net/~eoc/server-env-tools/6.1-mass_editing-fix_bug_1187937/+merge/167643 Your team OpenERP Community is subscribe

Re: [Openerp-community] [Merge] lp:~eoc/server-env-tools/6.1-mass_editing-fix_bug_1187937 into lp:~openerp-community/server-env-tools/6.1-mass_editing

2013-06-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~eoc/server-env-tools/6.1-mass_editing-fix_bug_1187937/+merge/167643 Your team OpenERP Community is subscribed to branch lp:~openerp-community/server-env-tools/6.1-mass_editing. _

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-mgmtsystem/7.0-set-default-risk-formula into lp:openerp-mgmtsystem

2013-06-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~openerp-community/openerp-mgmtsystem/7.0-set-default-risk-formula/+merge/166901 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-mgmtsystem/7.0-set-default-risk-formula. ___

[Openerp-community] [Merge] lp:~savoirfairelinux-openerp/openerp-product-attributes/product_unique_internal_reference into lp:openerp-product-attributes

2013-06-17 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~savoirfairelinux-openerp/openerp-product-attributes/product_unique_internal_reference into lp:openerp-product-attributes has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~savoirfairelinux-openerp/openerp-p

Re: [Openerp-community] [Merge] lp:~savoirfairelinux-openerp/openerp-product-attributes/product_unique_internal_reference into lp:openerp-product-attributes

2013-06-17 Thread Guewen Baconnier @ Camptocamp
Set as 'Work in progress', without response from the author since 2 months. -- https://code.launchpad.net/~savoirfairelinux-openerp/openerp-product-attributes/product_unique_internal_reference/+merge/156649 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. __

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/mixin-extraction into lp:openerp-product-attributes

2013-06-17 Thread Guewen Baconnier @ Camptocamp
l.78 the import from `tools.translate` should be from `openerp.tools.translate` once that fixed, I'll be ok for the merge -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/mixin-extraction/+merge/151333 Your team OpenERP Community is subscribed to branch lp:openerp-product

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/openerp-product-attributes-editable-views into lp:openerp-product-attributes

2013-06-14 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/openerp-product-attributes-editable-views/+merge/169235 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. __

[Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes

2013-04-29 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes has been updated. Status: Merged => Approved For more details, see: https://code.launchpad.net/~akretion-team/openerp-product-attributes/polymorphic-relations/+merge/1

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/mixin-extraction into lp:openerp-product-attributes

2013-04-29 Thread Guewen Baconnier @ Camptocamp
I tried to merge lp:~akretion-team/openerp-product-attributes/polymorphic-relations, then this branch, but when I merge this one I have a conflict. Can you check why please? -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/mixin-extraction/+merge/151333 Your team OpenERP

[Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes

2013-04-29 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~akretion-team/openerp-product-attributes/polymorphic-relations/+merge/1

[Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes

2013-04-29 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes has been updated. Status: Merged => Approved For more details, see: https://code.launchpad.net/~akretion-team/openerp-product-attributes/polymorphic-relations/+merge/1

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes

2013-04-29 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test LGTM Thanks for the change -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/polymorphic-relations/+merge/150725 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/mixin-extraction into lp:openerp-product-attributes

2013-04-29 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test Thanks for the change -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/mixin-extraction/+merge/151333 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Ma

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/mixin-extraction into lp:openerp-product-attributes

2013-03-19 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing I'm fine to merge this proposal without changing the nitpickings. But `openerp.osv.orm.setup_modifiers` has to be called on the fields. Raphaël, can you just change this please? The merge proposal can then be merged. By the way, that's really nice to have extracted this behav

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/polymorphic-relations into lp:openerp-product-attributes

2013-03-04 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing code review, no test s/osv.osv_memory/orm.TransientModel/ Same remark than the other merge proposal, you should call `setup_modifiers` when you add a node in the view. Otherwise, it seems fine to me. -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/p

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/mixin-extraction into lp:openerp-product-attributes

2013-03-04 Thread Guewen Baconnier @ Camptocamp
Review: Needs Information When you build a view yourself, you should call `openerp.osv.orm.setup_modifiers` on the nodes. (example in `account.wizard.account_report_common.account_common_report.fields_view_get`). In your case, I don't think it will change anything, because it setup the modifiers

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/no-stock-dep into lp:openerp-product-attributes

2013-02-18 Thread Guewen Baconnier @ Camptocamp
> When trying to merge this it brings in a lot more revisions than stated here. > Not sure what's going on here but it includes revisions made by Guewen, Maxime > and Alexis. > > These revisions seem to bring in a lot of new modules that are not ready for > v7 usage (still even using __terp__.py f

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/optional_attr_set into lp:openerp-product-attributes

2013-02-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve code review, no test A little nitpicking: l.92 I don't think you need to put a backslash here But it seems fine to me. -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/optional_attr_set/+merge/148603 Your team OpenERP Community is subscribed to branch lp:o

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/no-stock-dep into lp:openerp-product-attributes

2013-02-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve LGTM. -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/no-stock-dep/+merge/148515 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Mailing list: https://launchpad.net/~opene

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/dont-blow-if-same-field-twice into lp:openerp-product-attributes

2013-02-18 Thread Guewen Baconnier @ Camptocamp
Review: Approve Seems fine to me -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/dont-blow-if-same-field-twice/+merge/148606 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. ___ Mailing list: h

Re: [Openerp-community] [Merge] lp:~akretion-team/openerp-product-attributes/no-stock-dep into lp:openerp-product-attributes

2013-02-12 Thread Guewen Baconnier @ Camptocamp
When you create a new merge proposal, you can choose a prerequisite branch for your proposal. That's convenient because it does not display the diff of the prerequisite. -- https://code.launchpad.net/~akretion-team/openerp-product-attributes/no-stock-dep/+merge/147797 Your team OpenERP Community

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_images-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
Updated the proposal taking into account your comments. Still, I don't see cases where we are garanteed that 'name' and 'extension' are in the vals. When we do a create(), we are able to call it without any of them ('name' is mandatory, so the call to super() will raise an exception, but it sh

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_images-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
Hello, > we are double working here, Renato already did part of that work 2 weeks ago: > http://bazaar.launchpad.net/~extra-addons-commiter/product-extra- > addons/7.0/revision/67 The diff in your link was already there in the branch when I did my migration (the branch is as recent as the Sebast

Re: [Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_images-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
By the way, there is a LOT of room for improvements on this module. At least, it installs on v7, but it is far to be very good, in my opinion. -- https://code.launchpad.net/~camptocamp/openerp-product-attributes/7.0-product_images-migr/+merge/145630 Your team OpenERP Community is subscribed to bra

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_images-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-product_images-migr into lp:openerp-product-attributes with lp:~camptocamp/openerp-product-attributes/7.0-product_sequence-migr as a prerequisite. Commit message: [MIGR] migration of

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_sequence-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-product_sequence-migr into lp:openerp-product-attributes. Commit message: [MIGR] migration of product_sequence to openerp 7.0 Requested reviews: Product Core Editors (product-core-editors) For

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_m2mcategories-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
The proposal to merge lp:~camptocamp/openerp-product-attributes/7.0-product_m2mcategories-migr into lp:openerp-product-attributes has been updated. Commit Message changed to: [MIGR] migration of product_m2mcategories to openerp 7.0 For more details, see: https://code.launchpad.net/~camptocamp/

[Openerp-community] [Merge] lp:~camptocamp/openerp-product-attributes/7.0-product_m2mcategories-migr into lp:openerp-product-attributes

2013-01-30 Thread Guewen Baconnier @ Camptocamp
Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/openerp-product-attributes/7.0-product_m2mcategories-migr into lp:openerp-product-attributes. Requested reviews: Product Core Editors (product-core-editors) For more details, see: https://code.launchpad.net/~camptocamp/openerp

Re: [Openerp-community] [Merge] lp:~joao-gama/openerp-product-attributes/product_weight into lp:openerp-product-attributes

2013-01-20 Thread Guewen Baconnier @ Camptocamp
Hi Joao, The branch is now rebuild properly. Have a nice day Guewen -- https://code.launchpad.net/~joao-gama/openerp-product-attributes/product_weight/+merge/143710 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes. __

Re: [Openerp-community] [Merge] lp:~joao-gama/openerp-product-attributes/product_weight into lp:openerp-product-attributes

2013-01-18 Thread Guewen Baconnier @ Camptocamp
The 7.0 branch will have the same issue, the 'common ancestor' of the branches have to be the same than the extra-addons otherwise the merges won't work correctly. -- https://code.launchpad.net/~joao-gama/openerp-product-attributes/product_weight/+merge/143710 Your team OpenERP Community is subs

Re: [Openerp-community] [Merge] lp:~joao-gama/openerp-product-attributes/product_weight into lp:openerp-product-attributes

2013-01-18 Thread Guewen Baconnier @ Camptocamp
Review: Abstain Hello, There is a real problem with the target branch lp:openerp-product-attributes/6.1 I had to rebuild it entirely from the history of the other branches. You can read details here: https://code.launchpad.net/~product-core-editors/openerp-product-attributes/new6.1/+merge/1436

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-addons/fix-1100271-membership-invoice-form-view into lp:openobject-addons/7.0

2013-01-16 Thread Guewen Baconnier @ Camptocamp
Review: Approve -- https://code.launchpad.net/~openerp-community/openobject-addons/fix-1100271-membership-invoice-form-view/+merge/143504 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-addons/fix-1100271-membership-invoice-form-view. __

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-mgmtsystem/nc-extend into lp:openerp-mgmtsystem/6.1

2013-01-03 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing no test, review Hi, Thanks for your changes. You introduced a bug in your revision 53, you can't do that: 3122 + def case_reset(self, cr, uid, ids, context=None, *args): The keyword arguments should always be after the positional arguments. Your method will be parsed co

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-mgmtsystem/nc-extend into lp:openerp-mgmtsystem/6.1

2012-12-13 Thread Guewen Baconnier @ Camptocamp
Review: Needs Fixing Quick eyeball review, I did not reviewed the business aspect. dubious indentation at l. 647 837 + 'audit_ids': fields.many2many('mgmtsystem.audit','mgmtsystem_audit_nonconformity_rel','mgmtsystem_audit_id','mgmtsystem_action_id','Related Audits'), Just a remark, the re

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/import-from-addons-extra-trunk into lp:openerp-product-attributes/6.1

2012-12-10 Thread Guewen Baconnier @ Camptocamp
Oups! good catch! Thanks -- https://code.launchpad.net/~openerp-community/openerp-product-attributes/import-from-addons-extra-trunk/+merge/135473 Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes/6.1. ___ Mailing list: h

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/import-from-addons-extra-trunk into lp:~openerp-community/openerp-product-attributes/6.1

2012-12-10 Thread Guewen Baconnier @ Camptocamp
Review: Approve So, nobody disagree, I merge this branch without the product_gs1_128 module. -- https://code.launchpad.net/~openerp-community/openerp-product-attributes/import-from-addons-extra-trunk/+merge/135473 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-product-attributes/import-from-addons-extra-trunk into lp:~openerp-community/openerp-product-attributes/6.1

2012-12-04 Thread Guewen Baconnier @ Camptocamp
Hello, We have to merge this branch as soon as possible because the modules here are no longer present in the extra-addons branch. I am going to remove the product_gs1_128 module from the branch because it is now clear that it will be in lp:stock-logistic-barcode thanks to Numérigraphe. By the

Re: [Openerp-community] [Merge] lp:~openerp-community/web-addons/7.0-web_color into lp:~webaddons-core-editors/web-addons/7.0

2012-11-27 Thread Guewen Baconnier @ Camptocamp
Review: Approve That seems fine for me, thanks! -- https://code.launchpad.net/~openerp-community/web-addons/7.0-web_color/+merge/136214 Your team OpenERP Community is subscribed to branch lp:~openerp-community/web-addons/7.0-web_color. ___ Mailing lis

Re: [Openerp-community] [Merge] lp:~openerp-community/web-addons/7.0-web_color into lp:~webaddons-core-editors/web-addons/7.0

2012-11-27 Thread Guewen Baconnier @ Camptocamp
Yes, indeed, the `add` in the registry `instance.web.form.widgets` is totally necessary. I was surprised because I thought that (widget_name, 'openerp.web_color') should be (widget key, class of the widget). Here, the class of the widget seems to be the global namespace of the module instead of

Re: [Openerp-community] [Merge] lp:~openerp-community/web-addons/7.0-web_color into lp:~webaddons-core-editors/web-addons/7.0

2012-11-27 Thread Guewen Baconnier @ Camptocamp
Hi, Nice module. 76 + 'auto_install': True, It means that the module will be automatically installed once someone put the web-addons branch in its addons-path. I don't think that is advised for community branches. One may need to use the branch for another module, and he would have no cho

  1   2   >