Code review comment for lp:~rachidbm/ubuntu-l10n-tools/search

Revision history for this message
David Planella (dpm) wrote :

A couple of other things I've noticed (I'm still reviewing the 'search both' implementation):

* On ul10n_tools/search/TranslationsSearch.py, line 336 of the diff, please don't reformat the code. In general, when submitting merge proposals the changes should concentrate on the feature, so that the diff contains as little noise as possible.

* On ul10n_tools/search/TranslationsSearch.py, line 333, one thing that I really like is that you've managed to unify the code to have the same search loop for plural and singular entries (the "for entry_to_search in entries_to_search:" part. I think it might be interesting to explore optimizing this further and rewrite the for loop in a generic way that would work for both plural and singular, and have it called just once after the if clause in which we detect if the entry is plural or singular.

What do you think?

« Back to merge proposal