Mozilla.CompareLocales.FileCollector is a beast right now, using a generator function rather than just implementing a iterator object is much nicer to the eyes. Straight python-only thing, trying bhearsum for review on this one.
Created attachment 293664 [details] [diff] [review] use generator function instead of iterator object
Attachment #293664 - Flags: review?(bhearsum)
Comment on attachment 293664 [details] [diff] [review] use generator function instead of iterator object >+ leaf = dirpath + '/' + f You should use os.path.join here. Other than, this looks fine. r=bhearsum with that change.
Attachment #293664 - Flags: review?(bhearsum) → review-
Actually, that'd regress the expressions in filter.py, those just check for '/'. Note, I'm already ending up with forward slashes from the output of client.mk, the only code that inserts backslashes is os.walk, and I'd rather keep it that way. Or even work around that at one point.
On unix platforms, os.path.join will insert a '/', right? >>> a = "foo" >>> b = "bar" >>> import os.path >>> os.path.join(a, b) 'foo/bar' It sounds like this code is already unix specific, so I suppose it doesn't really matter that much.
Checked in. Note from irc, python on MozillaBuild uses '\', sadly.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Axel is good to see you got the bug already finished. I am not that good with python, I understand very basic concepts but I get lost with things like: - self.__w - self.__i --- I guess it is where you assign an iterator - self.__t --- it seems to have two arrays and some functionality, self.__t.sort() A one character variable, doesn't give too much idea but I can get a feeling of what they do. I will keep looking forward on learning python... (In reply to comment #0) > Mozilla.CompareLocales.FileCollector is a beast right now, using a generator > function rather than just implementing a iterator object is much nicer to the > eyes. > > Straight python-only thing, trying bhearsum for review on this one. > What do you mean by "generator function" rather than "implementing iterator"? If too long to explain, don't bother I will find out eventually by seing more code
Armen, You should look at the Generator and Iterator design patterns. Here's a start: http://en.wikipedia.org/wiki/Generator_(computer_science) http://en.wikipedia.org/wiki/Iterator
Armen, if you look at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/testing/tests/l10n/lib/Mozilla/CompareLocales.py&rev=1.11&mark=56-68#47, does that still look confusing? I think not so, which is why I removed all the cryptic state variables stuff in favour of a generator. The comparison of the two in the python docs is at http://docs.python.org/tut/node11.html#SECTION0011900000000000000000 (iterators), with the sexy stuff coming at http://docs.python.org/tut/node11.html#SECTION00111000000000000000000
+ for f in filenames: + leaf = dirpath + '/' + f + yield (leaf[cutoff:], leaf) Here is the yielding!! I got it, I like generators! Thanks both of you, I don't think I would have understood it properly if you didn't point where to look
Component: Testing → Infrastructure
Product: Core → Mozilla Localizations
QA Contact: testing → infrastructure
You need to log in before you can comment on or make changes to this bug.