----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/ -----------------------------------------------------------
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs -----
src/rolekit/server/rolebase.py b72071da1b0570f93dd31841747d0ea6967262da
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing -------
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/#review139 -----------------------------------------------------------
A matter of personal style; I’d be happier with no redundant comments (and trade them for more detailed docstrings), but I’ll go with the consensus.
- Miloslav Trmac
On Čec. 25, 2014, 9:02 odp., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/
(Updated Čec. 25, 2014, 9:02 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs
src/rolekit/server/rolebase.py b72071da1b0570f93dd31841747d0ea6967262da
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/#review141 -----------------------------------------------------------
I can't speak for Thomas, but my approach to coding is usually to write all or most of the comments for a function first and then go back and implement the code. Sometimes this does end up with seemingly-trivial comments, but I find that too many comments are better than too few (particularly if code edits down the line make things less obvious).
- Stephen Gallagher
On July 25, 2014, 7:02 p.m., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/
(Updated July 25, 2014, 7:02 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs
src/rolekit/server/rolebase.py b72071da1b0570f93dd31841747d0ea6967262da
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/ -----------------------------------------------------------
(Updated Čec. 28, 2014, 4:16 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs (updated) -----
src/rolekit/server/rolebase.py a32a95f852e00f88cae01e69d73e24cebcfe2aa8
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing -------
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/#review198 -----------------------------------------------------------
I'd prefer to just drop this patch, honestly. I've never really seen an issue with "too many comments".
- Stephen Gallagher
On July 28, 2014, 2:16 p.m., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/
(Updated July 28, 2014, 2:16 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs
src/rolekit/server/rolebase.py a32a95f852e00f88cae01e69d73e24cebcfe2aa8
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing
Thanks,
Miloslav Trmac
On Srp. 1, 2014, 1:22 odp., Stephen Gallagher wrote:
I'd prefer to just drop this patch, honestly. I've never really seen an issue with "too many comments".
I do find it annoying to have to read 3 lines instead of one, but that’s why it was a RFC; fair enough.
- Miloslav
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/#review198 -----------------------------------------------------------
On Srp. 1, 2014, 1:34 odp., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/
(Updated Srp. 1, 2014, 1:34 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs
src/rolekit/server/rolebase.py a32a95f852e00f88cae01e69d73e24cebcfe2aa8
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/47/ -----------------------------------------------------------
(Updated Srp. 1, 2014, 1:34 odp.)
Status ------
This change has been discarded.
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Comments to the tune of # Check values self.check_values(values)
IMHO only make the code less readable. Drop them.
Diffs -----
src/rolekit/server/rolebase.py a32a95f852e00f88cae01e69d73e24cebcfe2aa8
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/47/diff/
Testing -------
Thanks,
Miloslav Trmac
rolekit-commits@lists.stg.fedorahosted.org
-
Stephen Gallagher