Here's some general feedback as I work through the merge:
* Be careful about single quotes vs. double quotes. In general, I prefer single quotes except for multiline strings (including docstrings) and of course any string with an apostrophe in it.
* I fixed the interface for IDomainManager.add() - it still mentioned owners_id.
* You might want to use a Python linter to check for basic problems. E.g. I use pyflakes (hooked up to Emacs) so I noticed immediately some unused imports.
* Don't use mutables (e.g. []) in argument lists. If the mutable were to change, it would affect *every* call to the function. Use and check for `None` instead.
* You don't need to `assert isinstance(owners, list)` because we should accept any sequence. Probably not worth type checking the argument, as iteration ought to work, but of course it'll do the wrong thing if say owner is a string. That's okay, "we're all adults here" :) (Also, assert is not a function call so it doesn't need parentheses.)
* Implementations of interface methods can just have their docstring refer to the interface.
* I left a note in Domain.add_owners(). While I think the API is good, we might at some later point want to make that a bit more efficient than just calling add_owner(). Maybe.
Here's some general feedback as I work through the merge:
* Be careful about single quotes vs. double quotes. In general, I prefer single quotes except for multiline strings (including docstrings) and of course any string with an apostrophe in it.
* I fixed the interface for IDomainManager. add() - it still mentioned owners_id.
* You might want to use a Python linter to check for basic problems. E.g. I use pyflakes (hooked up to Emacs) so I noticed immediately some unused imports.
* Don't use mutables (e.g. []) in argument lists. If the mutable were to change, it would affect *every* call to the function. Use and check for `None` instead.
* You don't need to `assert isinstance(owners, list)` because we should accept any sequence. Probably not worth type checking the argument, as iteration ought to work, but of course it'll do the wrong thing if say owner is a string. That's okay, "we're all adults here" :) (Also, assert is not a function call so it doesn't need parentheses.)
* Implementations of interface methods can just have their docstring refer to the interface.
* I left a note in Domain. add_owners( ). While I think the API is good, we might at some later point want to make that a bit more efficient than just calling add_owner(). Maybe.
More to come as I continue with the merge.