Code review comment for lp://staging/~mterry/quickly/arb

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

\o/

The changes are looking good to me (and way better than the packaging insanity).

Some comments:
299 + print ("ERROR: Can't find", filepath)
you missed a %s or at least a space after "find" ;)

370 +print (project_version, template_version)
Is that a leftover?

379 + print("running", "find %s -name '*.py' -exec %s {} \;" % (python_name, sedline))
Hum, same here about print(a, b), this return a tuple, like print("bar", "foo")
('bar', 'foo')
not a friendly one line message, isn't it?

230 -import gettext
231 -from gettext import gettext as _
232 -gettext.textdomain('project_name')
233 +from locale import gettext as _
-> is it better? I have both in OneConf because of some UTF-8 errors and a fallback to the other one. If you have some info on that just for the info, I'll take them gladely :)

Thanks for adding the Exec=foo bar baz test case btw!

Approving (but I would appreciate some answers on my minor quesions before) ;) However, look at the pre-requisite branch which needs a little bit more work IMHO.

review: Approve

« Back to merge proposal