Code review comment for lp:~mlm-author/mailman/2.1-author

Sorry for not responding to this sooner. Somehow I overlooked the original Mailman-coders notification.

The branch covers almost everything, but it is incomplete:

Mailman.versions.NewVars() needs
        add_only_if_missing('author_list', 0)

Mailman.Version.DATA_FILE_VERSION needs to be incremented.

Most importantly, the code in Cleanse.py has issues:
+ # We change the from so the list takes ownership of the email
+ if mlist.author_list:
+ mlist.include_sender_header = 0
+ if msg['reply-to'] == "" :
+ msg['reply-to'] = msg['reply-to'] + " , " + msg['from']
+ else:
+ msg['reply-to'] = msg['from']

The above test is backwards and it doesn't take into account the fact that msg['reply-to'] is not a null string, but is None if there is no Reply-To: It should be something like:
        if msg['reply-to']:
            msg['reply-to'] = msg['reply-to'] + " , " + msg['from']
        else:
           msg['reply-to'] = msg['from']

+ realname, email = parseaddr(msg['from'])
+ del msg['from']
+ msg['From'] = formataddr(('%s via %s' % (realname,mlist.real_name), mlist.GetListEmail()))
+ del msg['domainkey-signature']
+ del msg['dkim-signature']

The above two headers and any Authentication-Results: header will be removed in current Mailman if REMOVE_DKIM_HEADERS is set to Yes in mm_cfg.py. You might want to rely on that instead of removing them here, particularly because your documentation in Defaults.py.in doesn't mention these.

+ del msg['sender']

In any case, Since this can be merged without changing existing behavior, I'll consider doing it for 2.1.16