openerp-community-reviewer team mailing list archive

["Holger Brunn \(Therp\)" <hbrunn@xxxxxxxx>]

Review: Needs Fixing

18: add spaces between operators

21: this seems to be one line too much

33: that's not safe against searching for '  / ' or the like, I'd wrap that into an
if category_names:

33, 36: shouldn't strip(product_category_separator + ' ') be enough? (the space just to be safe if we ever change product_category_separator)

42: if ids and classic_ids are nondistinct sets and there's a limit, you'll get less than limit ids, which is particularly annoying for name_search. I think you should pass limit+len(ids), remove duplicates in a manner that preserves class_ids' order and then shorten it as necessary

53: why is this necessary? And if so, why only if name is set?

63: can't you integrate that in the regex above? If you'll only have '[bla' trigger searching for the default code, but not 'bla]', adding a '?' after the closing square bracket would be enough. (and I can't really think of a use case for 'bla]').

64: if you don't agree with 63, .strip('[] ') should be enough here
-- 
https://code.launchpad.net/~yann-papouin/ocb-addons/6.1-bug-1267845-product-and-category-search-improved-usability/+merge/201192
Your team OpenERP Community Backports Team is subscribed to branch lp:ocb-addons/6.1.