banking-addons-team team mailing list archive

["Stefan Rijnhart \(Therp\)" <stefan@xxxxxxxx>]

Review: Needs Fixing

Thanks for the changes. I noticed the branch upload, although you did not set the status of this merge proposal to 'Needs review'. I will do that now. Please revert this setting if I am too quick.

Your changes are generally well done, but I still have a couple of remarks.

l.55,66 Typo: "bank statements" is written as two words.
l.350..356: the last revision introduces whitespace in this regular expression. Are you sure that this does not affect the matching of the data?
l.117,312: please attribute copyrights to Credativ as well. It is clear these are modified versions of the files from their HSBC module.

One more request for future reviews: I noticed you merged with the main branch in your last revision. That in itself is a good practice but if you do, could you commit this merge as a separate revision instead of committing your own changes in the same revision? It makes reviewing the changes since the last review a little easier.

-- 
https://code.launchpad.net/~endiansolutions/banking-addons/ab61-nl_rabo/+merge/141149
Your team Banking Addons Team is subscribed to branch lp:banking-addons.