Issues, issues, issues...

[Bardur Arantsson]

On 06/25/2015 06:32 PM, Francesco Ariis wrote:
> On Thu, Jun 25, 2015 at 05:55:47PM +0200, Bardur Arantsson wrote:
>> I noticed another thing while perusing the source code: There seem to be
>> quite a few TODO comments scattered about.
>>
>> Is there some sort of convention whereby it is permitted to add TODOs as
>> long as the person doing so pinkie-promises to fix/remove them later? Or
>> is it somehow related to severity? Or is it just that people couldn't be
>> bothered to make issues and just have them in out-of-band instead? Is it
>> all just legacy from before issue tracking was introduced? Do people
>> spontaneously clean up TODOs that others have left behind?
> 
> I recently released a little something [1] to scan for TODOs in a
> codebase (after being overwhelmed by them), so after your comment
> I felt compelled to see how cabal and its repo would fare!
> 

Oh, yes, I actually saw your announcement and should have remembered to
use it :).

Just to take a tiny sample from the top:

  40 Not sure about JHC
  41 This file should probably be removed.

Are these useful TODOs to have? Who (if anybody) is going to do anything
about them?

> I attach the output: there are a lot of todos but the number more or less
> on the same level with projects of similar complexity. I guess it is
> inevitable.

Unless you just ban them. Don't accept code which has TODOs in pull
requests -- it's really pretty simple According To Me(TM). Unless it's
your sole way of tracking issues, there are three categories of TODOs:

  1) The ones that are imporant enough to become Issues in a tracker.
     Put them in the tracker.

  2) The ones that aren't imporant enough to become Issues in a tracker.
     Remove them or change them to a little explanatory comment in case
     someone should find that place again during debugging. (Just in
     case you were wrong that it was a small/unimportant issue.)
     In some cases I find it more useful to put them in the commit
     comment as explanation -- commit messages *can* be a useful way to
     track such things. See e.g. Linux's general policy wrt. commit
     messages. (Granted, GitHub makes it harder to ensure commit
     message quality.)

  3) No, actually I think those two categories are probably enough.

That's it. Simple, as long as reviewers are willing to enforce the rule.

(Actually, I kind of lie. I don't think they should necessarily be
completely forbidden, but they *must* have a direct reference to an
*open* issue.in a tracker. This is just to make the reference from the
issue tracker to the relevant code more robust during unrelated changes
to the code, i.e. it just serves as a "marker" for robust linking. This
should ideally be checked automatically by periodic/on-pull-req code
validation.)

The reasoning for getting rid of TODOs entirely is simply that they are
another distraction that you don't need when maintaining code: Either
it's serious enough that it has to be addressed, or it's not serious
enough... in which case you shouldn't be distracted by it while
maintaining surrounding/adjacent code.

Regards,