← Back to team overview

banking-addons-team team mailing list archive

Re: lp:~akretion-team/banking-addons/bank-statement-reconcile-7.0-add-completion-label into lp:banking-addons/bank-statement-reconcile-7.0

 

Review: Needs Fixing code review, no tests

Hi,

Thanks for the contribs !

Few remarks:

 * lines 59-75 :

{
59	+ 'name': 'account_statement_completion_label',

=> Should be a "real english name" as displayed in the module list

60	+ 'version': '0.1',
61	+ 'category': 'Generic Modules/Others',
62	+ 'license': 'AGPL-3',
63	+ 'description': """empty""",

=> Should not be empty, please explain the behavior here cause it's not obvious for the end user how this new completion rule will work.

64	+ 'author': 'Akretion',
65	+ 'website': 'http://www.akretion.com/',
66	+ 'depends': ['account_statement_base_completion'],
67	+ 'init_xml': [],
68	+ 'update_xml': [
69	+ 'partner_view.xml',
70	+ 'statement_view.xml',
71	+ 'security/ir.model.access.csv',
72	+ ],

=> Deprecated, use 'data':

73	+ 'demo_xml': [],

=> Deprecated, use 'demo':

74	+ 'installable': True,
75	+ 'active': False,
76	+}


Otherwise, looks good to me. Please, make sure the description is clear enough for anybody.

Regards,

Joël

-- 
https://code.launchpad.net/~akretion-team/banking-addons/bank-statement-reconcile-7.0-add-completion-label/+merge/197758
Your team Banking Addons Core Editors is subscribed to branch lp:banking-addons/bank-statement-reconcile-7.0.