Managing TODO comments in SQL code

Transact-SQL
1

How do you tackle "TODO Driven Development"? The sort of thing I am thinking of is a noon-urgent / do-when-time type refinement to the code, not something that must be done before it will work at all.

I tend to put a comment in the code which is syntactically incorrect that thus will break, so that when I run the code (e.g. to create a stored procedure / trigger) I have to comment out the TODO. That way I at least get reminded that there are still unfinished items in the code every time I execute the code - because it is tedious to have to comment them out. When I check-in the code to revision control system I remove any such commenting-out's (I use a unique marker so I can globally replace easily), and that way anything commented-out gets reactivated, and has to be thought about again when the code is next executed.

It occurs to me that:

The comments are still in the SProcs, after creating, so I could report on them :slight_smile:

I could replace / augment the TODO comments with SProc calls - then real executing code would "log" all the embedded TODOs. That might help me with a "popularity" Stats report / graph

Other suggestions please?

2

At my last job, we had a policy where you ever left comments in code like that. Instead, everything was tracked in TFS. If there was an emergency code change to a stored proc that was temporary, a TFS issue was created to track it.

3

I agree. We do use an issue tracking system (for us that is Fogbugz, which we love ... other brands are available :slight_smile: ) and my first step, in "TODO Rehab"!!, should be to move anything out of TODO Comment into Fogbugz and then change the comment to "CASE #1234" to cross-ref it.

I can't make my mind up if it is a bad habit or a necessity! Clearly when I am coding something new and I think "I really ought to double check that XXX is true / valid / exists" it is easier to put a comment in, rather than break my train of thought - some of the TODOs are several hours work to resolve or need collaborative work to resolve. I suppose my reticence is that currently the TODO Comments are bugging me every time I try to run / modify that code. But ... they aint getting fixed any faster as just at the moment we are in a state of Charge!

However, your post has given me a thought:

As luck would have it :smiley: I have a Logging Sproc call at the top of every SProc source code file. It takes parameters for the SProc name and version number. It occurs to me that that Logging SProc could lookup the SProc in the Issue Tracking System and PRINT all open issues - so whenever I run the ALTER SProc script I would see whatever was still OPEN in Fogbugz ...

I think that would be the best-of-both-worlds, thanks for catalysing that thought Tara.

4

In case anyone is interest my LogScriptRun SProc works as follows:

Parameters:

  • SProc / Script name
  • Version - typically yymmdd - varchar(10)

Optional:

  • Action - NULL=Insert into log, "LIST"=Display history of Script Run
  • LastRun - 1=Show run dates (we have a log table of all SProc executions)

Running the Sproc with just SProc Name parameter defaults to displaying the history, so

EXEC my_SP_LogScriptRun 'MySprocName'     , '20160326'

will run the whole command when the script is executed (and log the CREATE SProc), but if I highlight just the EXEC and MySprocName I can get the Current Version and History

Actions:

Display warnings:

  • SProc parameter NULL/missing = display Command Help reminder
  • Running inside a transaction
  • Name contains invalid characters (according to our House Rules)
  • Check master.dbo.spt_values are at desired settings - e.g. ansi_nulls=ON, implicit_transactions=OFF

LIST mode:

If the object does not exist display a warning. There might still be entries in the History, but the object does not currently exist. (Thus the Sproc can be used as as useful "Exists?" check.

Modify Date is older then Script Log record. Because we do EXEC my_SP_LogScriptRun followed by ALTER/CREATE MySProc there is the possibility that the CREATE fails. We prefer to have the logging SProc at the top of the script, so it is easy to use for History List and to change the Version Number, but obviously it can't check that the Sproc exists at that point.

So to this I could add:

List any, open, records in Issue Tracking database :slight_smile:

Rollout:

During rollout (e.g. DEV to QA, and QA to PRODUCTION) we compare the latest entry, for each object in the log, between the two databases - which brings to light if we have run a script for the wrong version, or have an object present in one DB and not the other.

Usage:

PRINT 'Create procedure MySProcName' -- '
GO
EXEC dbo.my_SP_LogScriptRun 'MySprocName'     , '20160326'
GO
-- DROP PROCEDURE dbo.MySProcName
IF OBJECT_ID(N'[dbo].[MySProcName]', N'P') IS NULL
	EXEC ('CREATE PROC dbo.MySProcName AS SELECT ''MySProcName:stub version, to be replaced''')
GO

ALTER PROCEDURE dbo.MySProcName

... code here ...

GO
IF OBJECT_ID(N'[dbo].[MySProcName]', N'P') IS NOT NULL
	GRANT EXECUTE ON dbo.MySProcName TO MyRole
GO
PRINT 'Create procedure MySProcName DONE'
GO
5

I had a situation the other day where I stuffed some TODO's in the code - I'm interested to know how other folk would manage this.

All our tables have Create / Update DateTime & UserID columns.

We have a table where the users need to be able to see who first changed the status of the record to "Approved", so that table has an ApprovedBy column.

Recently the users said that they wanted to know the DateTime when the user approved the record, so we added ApprovedOn DateTime column with code in the trigger to populate ApprovedOn with the UpdateDateTime a) when the status changes to Approved and b) if ApprovedOn is NULL.(same logic is already used to populate ApprovedBy).

The onus was to start capturing this data, so the users had a useful history, and reporting it would come later.

As part of this job I searched for all instances of the usage of that table in the code,
to see if anything needed changing in the light of adding a new column to that table - however remote that possibility might be - and as part of that where relevant I added a TODO Comment "Consider displaying ApprovedOn"

I create a Ticket for the task of adding ApprovedOn to any relevant reports, and I could have documented all the instances in the code that I had found, but I don't think that would be as useful.

When I come to actually do the job the TODOs in the code are the only places I need to attend to for the fix (i.e. for any new code the column ApprovedOn will be considered as a matter of course, and there cannot be any old code that needs attention excepting the TODO locations I added).

The benefit that I perceive is that if, in the meantime, I fiddle with an Sproc that has one of those TODO comments I can choose to implement that at the same time as whatever else I am changing, and then eventually I'll get around to the actual task / ticket and sort out any TODOs that are left.