gtg-contributors team mailing list archive

[Lionel Dricot <ploum@xxxxxxxxx>]

Hi,

I 100% agree with Bryce on this.

I'm probably one of the most responsible for the commented print
statements.

The reason is simple : I don't use debug.log because I generally want only
informations about a very precise point. Then, when I have too many, I
quickly comment some of them out which means that I sometimes forget to
remove them before merging.

Sorry for that !

Lionel


On Tue, 3 Aug 2010 15:17:52 -0700, Bryce Harrington <bryce@xxxxxxxxxxxxx>
wrote:
> Hi all,
> 
> I've noticed all over the codebase there are lines of code commented
> out, but without an explanation why they're commented out.
> 
> I don't think this is a good practice.  It means that one of these
> happened:
> 
>  a.  I was unsure of my fix
>  b.  I wasn't sure what the original code was supposed to do
>  c.  I needed to disable it to work around some problem
>  d.  I removed or broke other code that this code depends on
>  e.  I started implementing something but haven't finished it yet
>  f.  I need this for debugging problems that might still exist
> 
> Obviously none of these are great situations to be in, but it happens.
> 
> Ideally, commenting out a line of code should be a signal to yourself
> that one of these things has happened, and that you probably should ask
> for help before merging it to trunk, and it should stay in a branch for
> now.
> 
> But that may not always be possible.  So more practically, when
> commenting out code please ALWAYS explain why you commented it out.
> This enables other developers (who may know the code better) to figure
> out and solve the problem.
> 
> The comments needn't be long.  Imagine it as a privmsg to a fellow dev,
> rather than as a personal reminder note which can be too terse for
> others to comprehend.  A 'FIXME' or 'TODO' label might be appropriate.
> 
> Also, if the issue is related to a bug, include the bug report number.
> If there isn't a bug report, consider filing one if you have time.
> 
> 
> A special case I see a lot are commented-out print statements for
> debugging.  In most cases something like this:
> 
>   #print "Testing: ", 1, 2, 3
> 
> can be better replaced with this:
> 
>   Log.debug("Testing %d %d %d", 1, 2, 3)
> 
> And if you don't think it's worth using Log.debug(), then it's probably
> not worth keeping the print statement at all!  :-)
> 
> 
> Going forward, I think we should turn a stern eye towards commented-out
> code.  Existing commented-out code should be reviewed and either removed
> or justified.  This will help make the codebase easier to understand and
> will help resolve the uncertainties / bugs / workarounds / incomplete
> works that the commented-out code implies.  It will also help tidy up
> the code.  
> 
> What do you guys think?
> 
> Bryce
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~gtg-contributors
> Post to     : gtg-contributors@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~gtg-contributors
> More help   : https://help.launchpad.net/ListHelp