Code review comment for lp:~akretion-team/account-invoicing/70-add-invoice_fiscal_position_update

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

Alexis,

Very well that when a user changes the fiscal position, there is the possibility to adapt the taxes and accounts.

Just wondering why the user should need to press a button. Should this not be an onchange action on the fiscal position?

As for the code itself it mostly seems good to me. Just two remarks:

1. Maybe better to avoid the line continuation with \ at #129 and #133 by using ( and )

2. The extremely long lines in the xml at #168 and #185 might be avoided by having each attribute on its own line.

So for the moment I abstain on this proposal.

review: Abstain

« Back to merge proposal