This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/66/

On August 19th, 2014, 2:56 p.m. UTC, Miloslav Trmac wrote:

config/roles/testrole/role.py (Diff revision 1)
def do_update(self, sender=None):
105
105
106
    # Check own properties

Documentation IMHO belongs into RoleBase, not to be copy&pasted in all roles. If necessary, attach it to to a method that only raises NotImplemeted (or returns False?)

Also, please keep the interfaces of all roles in sync (i.e. when adding a method to one, add it to all.)

On August 19th, 2014, 7:49 p.m. UTC, Miloslav Trmac wrote:

And finally, having this a docstring instead of a comment would be nice. Tools understand docstrings.

I think that a writer of a role should have an example role, that is explaining all the things he needs to add and also in which way. If a role writer has to look in RoleBase he will find a lot of stuff that might not be of interest in creating a role. But RoleBase might be good as a reference if there are open questions.

The role itself does not necessarily care a lot about D-Bus stuff.


- Thomas


On August 18th, 2014, 12:03 p.m. UTC, Thomas Woerner wrote:

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
By Thomas Woerner.

Updated Aug. 18, 2014, 12:03 p.m.

Repository: rolekit

Description

This is needed to be able to check types of role specific properties.

Diffs

  • config/roles/testrole/role.py (358deca3fc7172929d53d2c77efd5c919da2aea9)
  • src/rolekit/server/rolebase.py (8591f8aade76d3463647c59b43cc83877698182d)

View Diff