Code review comment for lp:~pascal-devuyst/apt-ddtp-tools/DebianSync

Hello Pascal,

thanks a lot for this branch and sorry for my slow reply.

This looks good, but I do have some questions:

=== modified file 'po2translation.py'
--- po2translation.py 2010-12-20 13:22:27 +0000
+++ po2translation.py 2013-05-30 07:21:40 +0000
@@ -41,7 +41,7 @@
                # we have no translation for this chunk
                if only_do_full_translations:
                        return ""
- short_descr_trans = short_descr
+ short_descr_trans = "<trans>"
        else:
                have_translation = True

Why is this needed? Will this <trans> get filtered out later?
@@ -91,7 +91,7 @@
                        for line in chunk.split("\n"):
                                if line:
                                        new_chunk += " %s\n" % line
- chunk = new_chunk
+ chunk = new_chunk[:-1] # get rid of trailing \n
                # make sure here that chunks are seperated with "\n .\n"
                if long_descr_str == "":
                        long_descr_str = "%s" % chunk

Its probably best to use chunk = new_chunk.rstrip("\n") here to be robust against chunks that do not end with \n.

For the sending of mails, I wonder if it wouldn't be easier to just call the sendmail binary (maybe as a optional way) for people who already have a installed and configured sendmail.

It would be nice if the return codes (253, 252 etc) also have symbolic names somewhere.

I hope the above makes sense, please let me know if you have any questions. Its a nice touch that you also fix the hardcoded ubuntu mirror and make that configurable instead, its only a small part of the large patch but still a nice touch :)

Out of curiosity, when we send new translations to the email interface, do we have to be careful about not sending too much? Will it enter some sort of queue/review process in debian? Should we notify them before we send the stuff back or did you already started the sending back ?

Thanks!
 Michael