----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/ -----------------------------------------------------------
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs -----
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review79 -----------------------------------------------------------
Not a full review at this point.
src/rolectl http://reviewboard-fedoraserver.rhcloud.com/r/30/#comment65
Undefined variable msg
src/rolectl http://reviewboard-fedoraserver.rhcloud.com/r/30/#comment67
What is “list” used for?
src/rolectl http://reviewboard-fedoraserver.rhcloud.com/r/30/#comment66
Would be cleaner with an explicitly passed parameter for “a”.
src/rolectl http://reviewboard-fedoraserver.rhcloud.com/r/30/#comment68
Can I set non-string settings?
N
- Miloslav Trmac
On Čec. 22, 2014, 12:25 odp., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated Čec. 22, 2014, 12:25 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
On July 22, 2014, 1:51 p.m., Miloslav Trmac wrote:
src/rolectl, line 89 http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/1/?file=108#file108line89
What is “list” used for?
Shoul dbe used instead of a.set here
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review79 -----------------------------------------------------------
On July 22, 2014, 10:25 a.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated July 22, 2014, 10:25 a.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
On July 22, 2014, 1:51 p.m., Miloslav Trmac wrote:
src/rolectl, line 96 http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/1/?file=108#file108line96
Can I set non-string settings?
For keys we do not support it. For values, this will be added with the json patch
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review79 -----------------------------------------------------------
On July 22, 2014, 10:25 a.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated July 22, 2014, 10:25 a.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
On July 22, 2014, 1:51 p.m., Miloslav Trmac wrote:
src/rolectl, line 96 http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/1/?file=108#file108line96
Can I set non-string settings?
Thomas Woerner wrote: For keys we do not support it. For values, this will be added with the json patch
keys in dicts are strings in D-Bus
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review79 -----------------------------------------------------------
On July 22, 2014, 10:25 a.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated July 22, 2014, 10:25 a.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/ -----------------------------------------------------------
(Updated July 22, 2014, 3:09 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs (updated) -----
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review110 -----------------------------------------------------------
src/rolectl http://reviewboard-fedoraserver.rhcloud.com/r/30/#comment83
You reuse this everywhere. While I realize it's only two lines, that much exact duplication is wasteful.
Please just create an instance_obj up at the top before the if/else and just consume it in each of the conditionals.
src/rolectl http://reviewboard-fedoraserver.rhcloud.com/r/30/#comment84
s/uncatched/uncaught/
- Stephen Gallagher
On July 22, 2014, 3:09 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated July 22, 2014, 3:09 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/ -----------------------------------------------------------
(Updated July 22, 2014, 6:13 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs (updated) -----
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review113 -----------------------------------------------------------
Ship it!
Ship It!
- Stephen Gallagher
On July 22, 2014, 6:13 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated July 22, 2014, 6:13 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
On July 22, 2014, 6:22 p.m., Stephen Gallagher wrote:
Ship It!
Pushed: https://git.fedorahosted.org/cgit/rolekit.git/commit/?id=25802294fec720fae13...
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/#review113 -----------------------------------------------------------
On July 22, 2014, 6:13 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/
(Updated July 22, 2014, 6:13 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/30/ -----------------------------------------------------------
(Updated July 22, 2014, 7:14 p.m.)
Status ------
This change has been marked as submitted.
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Provides commands: version, list, settings, deploy, redeploy, status, start, stop, restart, update and decommission
Diffs -----
src/rolectl 47044818ee66f0f12af4682ef19360f85db89180
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/30/diff/
Testing -------
Thanks,
Thomas Woerner
rolekit-commits@lists.fedorahosted.org