[Mailman-Developers] [CLI Project] Introduced Unit tests to CLI

[Rajeev S]

On Mon, Jun 2, 2014 at 11:53 AM, Abhilash Raj <raj.abhilash1 at gmail.com>
wrote:

> Hi Rajeev,
>
> Rajeev S <rajeevs1992 at gmail.com> wrote:
>
> Thanks for considering my suggestion. I think you don't need to create
> a sub-module for each scope inside `tests` module. It should be simpler
> like `/tests/test_domain.py` and all the tests related to domains
> should reside inside that. You can add multiple test-cases for Domain
> like add, delete or whatever inside that single test_domain.py.
>
>

I was in an idea to follow "the one class per file" style, especially when
the classes
tend to grow large in size as in the tests.(It was mentioned in the Barry's
style guide, and so is a common coding guideline.)

Should I change that?



>
> I see that you parsing the configuration to return a
> `mailmanclient.Client()` object twice, once in cmdparser.py and second
> time in`/lib/mailman_utils.py`. I don't understand why? Can
> cmdparser.py not use the `MailmanUtils.connect()` method?
>
>

cmdparser.py takes the login credentials from the command line, and not
the config.ini. config.ini, as of now, is just for the testing stuff.



Here :/src/mailmanclient/cli/core/domains.py#L32 docstrings should be
> immediately after the `def` line, there should not be a empty-line in
> between them.
>
> Here: /src/mailmanclient/cli/core/users.py#L41 you can use upto 80chars
> in docstrings too before introducing a line break.


>
Also if you are following the convention of specifying the parameter of
> a function in docstring, add a `:type param: foo` for each parameter
> too. It helps sphinx in generating better documentation.
> See this:
>   https://pythonhosted.org/an_example_pypi_project/sphinx.html




Fine.Will fix with the next revision.




> Everything else looks nice.
>
>

Thank you!