Merge lp:~initos.com/openerp-connector-magento/7.0-small-kwargs-improvement-sale into lp:~openerp-connector-core-editors/openerp-connector-magento/7.0

[Thomas Rehn]

The method 'search' of the SaleOrderAdapter uses keyword arguments. This code branch changes the call so that is uses keyword arguments. This allows inheriting classes to be lazy and do something like

def search(self, filters=None, **kwargs):
    # add state to filters
    filters = filters or {}
    filters['state'] = 'complete'
    return super(mySaleOrderAdapter, self).search(filters=filters,
        **kwargs)

instead of explicitly writing down all other keyword arguments.

I know that there are some other places in the code where functions are not called by keyword. With this merge proposal I want to start a discussion if such a change goes in the right direction. Its obvious disadvantage is that it makes the call a bit more cumbersome.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

I totally agree that keyword arguments should be called using the keyword.

I have just one remark about pep8 regarding the indentation, it says "Arguments on first line forbidden when not using vertical alignment" [0]

So I propose to write

    record_ids = self.backend_adapter.search(
        filters=filters,
        from_date=from_date,
        magento_storeview_ids=magento_storeview_ids)

[0] ref: http://legacy.python.org/dev/peps/pep-0008/#indentation

[lp:~initos.com/openerp-connector-magento/7.0-small-kwargs-improvement-sale]

1004. By Thomas Rehn on 2014-06-18

make change pep-8 compliant

[Thomas Rehn (thomas-rehn)]

Thanks for the hint. I improved the code in my branch. I will keep my eyes open for other keyword-function calls that could be improved.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Thanks!

review: Approve (code review)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

This project is now hosted on https://github.com/OCA/connector-magento. Please move your proposal there. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

review: Needs Resubmitting

Unmerged revisions

1004. By Thomas Rehn on 2014-06-18

make change pep-8 compliant

1003. By Thomas Rehn on 2014-06-18

keyword arguments should be called as such

=== modified file 'magentoerpconnect/sale.py'
--- magentoerpconnect/sale.py	2014-05-26 11:25:41 +0000
+++ magentoerpconnect/sale.py	2014-06-18 14:20:45 +0000
@@ -273,9 +273,10 @@
         filters['state'] = {'neq': 'canceled'}
         from_date = filters.pop('from_date', None)
         magento_storeview_ids = [filters.pop('magento_storeview_id')]
-        record_ids = self.backend_adapter.search(filters,
-                                                 from_date,
-                                                 magento_storeview_ids)
+        record_ids = self.backend_adapter.search(
+            filters=filters,
+            from_date=from_date,
+            magento_storeview_ids=magento_storeview_ids)
         _logger.info('search for magento saleorders %s  returned %s',
                      filters, record_ids)
         for record_id in record_ids: