Code review comment for lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-fixes

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

By openerp <openerp@oerp70> on 2013-05-13
a couple of fixes

Don't you want to set the right email in your bzr whoami to be properly linked on launchpad ?
Plus could you use [FIX] or [ADD] tag on your commit message ?

Otherwise, for other change needed:

l43 + l48 Use orm.Model instead of osv.osv

l62 can be removed as not needed anymore since v6.1

l976 - 978 added comment here seems strange

Furthermore, about point 2, is there a bug report on launchpad about it ? Would be great to have one and link your commit to it by using parameter --fixes lp:<bug number>

As you might want to resubmit your MP to fix the commit message and author, I think it would be better to separate those 3 points in 3 different merge proposals. So I set you MP as "Resubmit" so you can fix those.

Thanks for contributing, and looking forward for those few improvements

review: Needs Resubmitting

« Back to merge proposal