[Mailman-Developers] [CLI Project] Added Backup and Restore Tool

[Stephen J. Turnbull]

Rajeev S writes:

 > I feel that it will make the code cluttered. Since the CLI code
 > is independent of the rest of mailman client, won't it be
 > better to maintain the CLI code in a separate folder, as it is now?

Of course.  I didn't explain myself well.  What you have now is

mailman/ -+- client/ -+- _client.py
                      +- docs/
                      +- tests/
                      +- cli/ -+- client.py
                               +- core/
                               +- docs/
                               +- lib/
                               +- tests/

and what I would like to see is something like

mailman/ -+- client/ -+- _client.py
          |           +- docs/
          |           +- tests/
          +- cli/ -+- client.py
                   +- core/
                   +- docs/
                   +- lib/
                   +- tests/

It's only one level higher, but it makes the relationship
(mailman.client provides services to mailman.cli) clearer, and
emphasizes the user app (cli).

 > > Is there any code outside of this directory that is yours?
 > 
 > I have made modifications to setup.py, other than that, no.

OK, that's great!

 > > It's not clear to me why you define a separate main() function in
 > > this script -- it doesn't make sense to import it as a module.
 >
 > I was not quite familiar with using the python setuptools and I
 > assumed that such a format was necessary to specify module entry
 > points in setup.py. I have now removed the main function from
 > mmclient.py

OK.  It's up to you (and maybe Barry has a stylistic opinion on it).

 > Yikes! Sorry about that. Fixed at least ten [typos] :)

And I thought I was being a cranky old man.  Glad we caught those. :-)

 > > In lib/utils.py class Filter, what are the class attributes for?  They
 > > are not used or useful.
 > 
 > The Filter class is used to filter out objects [...]
 > 
 > All the class attributes are being used in the process described above.

I don't see where.  In Filter, you have:

class Filter():
    key = None
    value = None
    operator = None
    data_set = []
    utils = Utils()

    def __init__(self, key, value, operator, data):
        self.key = key
        self.value = value
        self.operator = operator
        self.data_set = data

So as soon as you instance a Filter, __init__() creates instance
attributes which shadow the class attributes (except utils).  I don't
see any references to Filter.<attr> or to super(), so I don't see how
you can be using the class attributes.

 > Now that the connection part is managed by a single function
 > (Utils.connect), I will move the connection checking part to that
 > function.

OK.

 > Those lists were not stored into variables until some revisions back. I got
 > a random thought that it is a bad practice , and replaced that code with
 > a variable assignment. It was then when my PEP8 checker returned an error
 > stating the presence of an unused variable, which I solved by deleting the
 > assigned variable!

;-)

 > I will look into the possibility of refactoring in those parts, I
 > have some plans in mind. Hope that those work!

That would be very cool!  As I say, I'm not confident that it's worth
the work, so be careful about wasting your time just because *I* said
it.  (It's not a waste of time if you do it because you think it's an
interesting problem.)

 > All the mentioned changes (except the tests) would be completed at
 > the latest, by 9th Aug 2014, Saturday. (r 70) Planning to spend the
 > weekend on writing new unit tests. (r 71)

Sounds good!