Code review comment for lp://staging/~mayankjuneja/systers/review-testing

Revision history for this message
Robin J (robin-jeffries) wrote :

=== modified file 'Mailman/DlistMemberships.py'
--- Mailman/DlistMemberships.py 2009-08-23 16:58:30 +0000
+++ Mailman/DlistMemberships.py 2012-06-02 18:11:19 +0000
@@ -11,7 +11,7 @@
 from Mailman import MemberAdaptor
 #from mm_cfg import DEBUG_MODE

-ISREGULAR = 1
+ISREGULAR_MODIFIED= 1
 ISDIGEST = 2

 Isn't there a conflict between ISREGULAR_MODIFIED = 1 and ISDIGEST=2? how can both be true?
This is an example of changing a line. It shows as a delete and an add

=== modified file 'Mailman/DlistUtils.py'
--- Mailman/DlistUtils.py 2010-05-27 18:01:32 +0000
+++ Mailman/DlistUtils.py 2012-06-02 18:11:19 +0000
@@ -848,8 +848,3 @@
    if alreadyLocked == 0:
        mlist.Unlock()

-def remove_dlist(listname):
- """ Deletes the corresponding postgres database and tables."""
- _remove_database(listname)
- if DEBUG_MODE:
- syslog('info', "Database %s removed\n", listname)

One of my frustrations here is that I can't tell if you removed the entire function (it would be bad if you left the last line hanging around).
This is an example of removing lines

=== modified file 'Mailman/ListAdmin.py'
--- Mailman/ListAdmin.py 2009-01-29 18:54:13 +0000
+++ Mailman/ListAdmin.py 2012-06-02 18:11:19 +0000
@@ -45,6 +45,9 @@
 from Mailman.Logging.Syslog import syslog
 from Mailman import i18n

+#Dummy additions
+import commands
+

The 3 lines above are what was added. Clear, but probably the use of the commands imported would be far away from this, and I would be confused by the time I got to it.

 _ = i18n._

 # Request types requiring admin approval

=== added file 'Mailman/dummy_file.py'
--- Mailman/dummy_file.py 1970-01-01 00:00:00 +0000
+++ Mailman/dummy_file.py 2012-06-02 18:11:19 +0000
@@ -0,0 +1,1 @@
+Dummy File

An empty (?) file was added (that is, it was checked into the branch, right?)

review: Disapprove

« Back to merge proposal