On srpen 19th, 2014, 4:56 odp. CEST, 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 srpen 19th, 2014, 9:49 odp. CEST, Miloslav Trmac wrote:

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

On srpen 20th, 2014, 7:48 odp. CEST, Thomas Woerner wrote:

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.

IMHO the API should have a documentation in exactly a single place, and that’s a docstring within RoleBase (perhaps in an “abstract method”).
Examples and the like should ideally not be necessary beyond the API documentation, but even if so, please keep the other roles in sync at least WRT the non-comment lines (we are “writers" of those roles).  Both me and Stephan have already ended up copy&pasting testrole into the other roles because they were not in sync; this was feasible because the other roles had no content, but now that they do, having to manually review the changelog of testrole and see which commits to mirror in the other roles is unnecessary work (and one wouldn’t even notice that the work is necessary, the roles will just break if not updated).

- Miloslav

On srpen 20th, 2014, 8:02 odp. CEST, Thomas Woerner wrote:

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