Hi Rajeev,

Rajeev S <rajeevs1992@gmail.com> wrote:

Hi,

As per Abhilash's comment to follow the TDD, I have covered the test cases for the domain scope.

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 know that TDD requires to write tests before implementation. However, I believe that it's not a good idea to postpone test cases for the existing units further.

Also, I would like to thank Abhilash for the comment, before its too late to begin writing the tests.

Glad I could help :)

I have pushed the r56 that includes the test scripts.

I have also done some significant refactoring on the code this weekend, to facilitate easy usage with the unittest package and to make the code more cleaner. The refactoring mainly includes imposing the of DRY principle -- moving repetitive code to the lib/* directory.

As an end note, this is a hefty revision. :)

http://bazaar.launchpad.net/~rajeevs1992/mailman.client/mailmancli/revision/...

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?

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

Everything else looks nice.

-- thanks, Abhilash Raj

Hi Rajeev,

Rajeev S <rajeevs1992@gmail.com> wrote:

Hi,

As per Abhilash's comment to follow the TDD, I have covered the test cases for the domain scope.

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 know that TDD requires to write tests before implementation. However, I believe that it's not a good idea to postpone test cases for the existing units further.

Also, I would like to thank Abhilash for the comment, before its too late to begin writing the tests.

Glad I could help :)

I have pushed the r56 that includes the test scripts.

I have also done some significant refactoring on the code this weekend, to facilitate easy usage with the unittest package and to make the code more cleaner. The refactoring mainly includes imposing the of DRY principle -- moving repetitive code to the lib/* directory.

As an end note, this is a hefty revision. :)

http://bazaar.launchpad.net/~rajeevs1992/mailman.client/mailmancli/revision/...

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?

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

Everything else looks nice.

-- thanks, Abhilash Raj