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.
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
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/66/#review230
-----------------------------------------------------------
On Srp. 20, 2014, 8:02 odp., Thomas Woerner wrote:
-----------------------------------------------------------
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