Re: Review Request 66: New support for do_check_property

On Srp. 19, 2014, 4:56 odp., Miloslav Trmac wrote: > config/roles/testrole/role.py, lines 105-106 > <http://reviewboard-fedoraserver.rhcloud.com/r/66/diff/1/?file=259#file259... > > 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.) Miloslav Trmac wrote: And finally, having this a docstring instead of a comment would be nice. Tools understand docstrings. 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. Miloslav Trmac wrote: 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).

----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/66/ ----------------------------------------------------------- (Updated Srp. 20, 2014, 8:02 odp.) Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner. Repository: rolekit Description ------- This is needed to be able to check types of role specific properties. Diffs ----- config/roles/databaseserver/role.py 63687c0be4a5705e772f1a6d60932f69ce9850e7 config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 config/roles/testrole/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/server/rolebase.py cd28d6a47cf183f6017905034f178d7aecf83348 Diff: http://reviewboard-fedoraserver.rhcloud.com/r/66/diff/ Testing ------- Thanks, Thomas Woerner