----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/196/#review481 -----------------------------------------------------------
src/rolekit/server/io/rolesettings.py (line 43) http://reviewboard-fedoraserver.rhcloud.com/r/196/#comment278
Typo: "conenct" -> "connect"
src/rolekit/server/io/rolesettings.py (lines 52 - 55) http://reviewboard-fedoraserver.rhcloud.com/r/196/#comment279
Wouldn't it be more "pythonic" to do:
``` try: self._callbacks[signal] = (handler, args) except KeyError: raise ValueError("Unknown signal name '%s'" % name) ```
src/rolekit/server/io/rolesettings.py (lines 68 - 69) http://reviewboard-fedoraserver.rhcloud.com/r/196/#comment281
Similarly, isn't it more pythonic for this to be:
``` try: cb = self._callbacks["changed"] except KeyError: # Log the lack of a changed callback return cb_args = [ key, value ] ... ```
src/rolekit/server/io/rolesettings.py (lines 72 - 74) http://reviewboard-fedoraserver.rhcloud.com/r/196/#comment282
``` try: cb_args.extend(cb[1]) except TypeError: # Got None here pass ```
- Stephen Gallagher
On Aug. 26, 2015, 1:21 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/196/
(Updated Aug. 26, 2015, 1:21 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
Only the changed signal is supported right now. As soon as a settings is modified, the connected handler will be called for this setting.
Diffs
src/rolekit/server/io/rolesettings.py 18f4871faf87ba23d21a3f9bc2c13b14463a8ccf
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/196/diff/
Testing
Thanks,
Thomas Woerner