----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/ -----------------------------------------------------------
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs -----
src/rolekit/server/dbusrole.py 8a13ccccbb0c9d2940f485697d9d2ef644183b15 src/rolekit/server/roled.py 59d1523347325b20d666c99d8c2087c8ce486608
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/#review191 -----------------------------------------------------------
src/rolekit/server/dbusrole.py http://reviewboard-fedoraserver.rhcloud.com/r/55/#comment106
What does “private” mean in this context BTW? It's obviously neither class-private nor file-private.
src/rolekit/server/dbusrole.py http://reviewboard-fedoraserver.rhcloud.com/r/55/#comment105
No. Logging and ignoring exceptions deep within the call stack, and returning None from functions that are not documented to return None, is not a reasonable error handling strategy.
(I’m not working on fixing the other places, but let’s not spread this any further.)
src/rolekit/server/roled.py http://reviewboard-fedoraserver.rhcloud.com/r/55/#comment107
This kind of suggests that ._roles should be a dictionary instead… but that’s for another time.
- Miloslav Trmac
On Čec. 30, 2014, 5:33 odp., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/
(Updated Čec. 30, 2014, 5:33 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs
src/rolekit/server/dbusrole.py 8a13ccccbb0c9d2940f485697d9d2ef644183b15 src/rolekit/server/roled.py 59d1523347325b20d666c99d8c2087c8ce486608
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing
Thanks,
Thomas Woerner
On Čec. 31, 2014, 4:44 odp., Miloslav Trmac wrote:
src/rolekit/server/dbusrole.py, line 209 http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/1/?file=238#file238line209
No. Logging and ignoring exceptions deep within the call stack, and returning None from functions that are not documented to return None, is not a reasonable error handling strategy. (I’m not working on fixing the other places, but let’s not spread this any further.)
And AFAICT it is completely unnecessary as well; this method just can’t fail.
- Miloslav
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/#review191 -----------------------------------------------------------
On Čec. 30, 2014, 5:33 odp., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/
(Updated Čec. 30, 2014, 5:33 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs
src/rolekit/server/dbusrole.py 8a13ccccbb0c9d2940f485697d9d2ef644183b15 src/rolekit/server/roled.py 59d1523347325b20d666c99d8c2087c8ce486608
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing
Thanks,
Thomas Woerner
On July 31, 2014, 2:44 p.m., Miloslav Trmac wrote:
src/rolekit/server/dbusrole.py, lines 206-207 http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/1/?file=238#file238line206
What does “private” mean in this context BTW? It's obviously neither class-private nor file-private.
Private means here that is it not visible via D-Bus.
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/#review191 -----------------------------------------------------------
On July 30, 2014, 3:33 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/
(Updated July 30, 2014, 3:33 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs
src/rolekit/server/dbusrole.py 8a13ccccbb0c9d2940f485697d9d2ef644183b15 src/rolekit/server/roled.py 59d1523347325b20d666c99d8c2087c8ce486608
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/ -----------------------------------------------------------
(Updated Aug. 20, 2014, 5:40 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Bugs: 3 https://fedorahosted.org/rolekit/ticket/3
Repository: rolekit
Description -------
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs (updated) -----
src/rolekit/server/dbusrole.py 51c791c236f79855a5be5eb6bc5108c37c218af4 src/rolekit/server/roled.py 1f17e4fa27a8dd16e80c1b82b2b9338d02a3e587
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/ -----------------------------------------------------------
(Updated Aug. 20, 2014, 7:02 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Bugs: 3 https://fedorahosted.org/rolekit/ticket/3
Repository: rolekit
Description -------
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs -----
src/rolekit/server/dbusrole.py 51c791c236f79855a5be5eb6bc5108c37c218af4 src/rolekit/server/roled.py 1f17e4fa27a8dd16e80c1b82b2b9338d02a3e587
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/#review254 -----------------------------------------------------------
Ship it!
Ship It!
- Miloslav Trmac
On Srp. 20, 2014, 9:02 odp., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/
(Updated Srp. 20, 2014, 9:02 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Bugs: 3 https://fedorahosted.org/rolekit/ticket/3
Repository: rolekit
Description
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs
src/rolekit/server/dbusrole.py 51c791c236f79855a5be5eb6bc5108c37c218af4 src/rolekit/server/roled.py 1f17e4fa27a8dd16e80c1b82b2b9338d02a3e587
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/55/ -----------------------------------------------------------
(Updated Aug. 21, 2014, 1:13 a.m.)
Status ------
This change has been marked as submitted.
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Bugs: 3 https://fedorahosted.org/rolekit/ticket/3
Repository: rolekit
Description -------
Fixes https://fedorahosted.org/rolekit/ticket/3
Diffs -----
src/rolekit/server/dbusrole.py 51c791c236f79855a5be5eb6bc5108c37c218af4 src/rolekit/server/roled.py 1f17e4fa27a8dd16e80c1b82b2b9338d02a3e587
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/55/diff/
Testing -------
Thanks,
Thomas Woerner
rolekit-commits@lists.stg.fedorahosted.org
-
Stephen Gallagher