Issues, issues, issues...
spam at scientician.net
Thu Jun 25 16:56:34 UTC 2015
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  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
> 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
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
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
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.
More information about the cabal-devel