Why you should do code reviews for sysadmin scripts

July 27, 2009

Through my experiences over the past while, I've come around to the view that you should try to have code reviews for sysadmin shell scripts. There's two reasons for this, and they both have to do with the fact that the Bourne shell is not really a programming language.

First, you want code reviews so that other people can convince you that your clever Bourne shell idioms are a little too clever. People's tastes and standards for this vary widely, and you're writing scripts not just for yourself but for your co-workers as well; black-box scripts (ones that no one but you can touch) ultimately don't help anyone.

(And some times you have strong disagreements over the best way to do something and need to come to an agreement on what the local style will be.)

Second and more importantly, because there is a lot of Bourne shell arcana (and beartraps) that people don't know, especially junior people. There are all sorts of clever but not obvious ways of doing things, or conditions that it's not obvious that you need to handle. If you don't know the particular trick that you need, you wind up writing shell scripts that do things inefficiently or miss cases or have subtle bugs.

(It's not just 'junior people' who miss shell idioms; for example, I've learned a number of new-to-me ones through WanderingThoughts, both in writing entries and in the comments people have written.)

Code review of people's scripts gives you an opportunity to pass on these tricks and arcana (and in a way that people may find easier to remember than some great big list of 'nifty shell tricks'), and in the process you improve the quality of your local scripts. As a bonus, you may even learn new ones yourself.

(This is especially good if it means that you fix an overlooked condition in a script before it goes off in someone's face. No one likes to make a mistake in a script that causes problems in production, and it's probably especially demoralizing for junior people and, to put it one way, not a great way to convince them that they're competent to write scripts and should keep on doing so.)


Comments on this page:

From 71.250.234.178 at 2009-07-27 10:43:49:

Can I add a plea for people who have been asked to review others' scripts to actually review it?

I recently wrote a script to do an oracle copy from a standby machine to another, then activate it. I passed it on to have someone review it, and they cleared it.

In the script, I had very elegant checking to make sure that the current database was up to date, and that the remote database was shut down, and my remote activation code was beautiful.

Unfortunately, I left out the whole "shut down the local database" part. And the other guy didn't catch it either.

Whoops.

It's still my fault, but regardless, this past weekend, a database copy wasn't made, and I'm in the middle of doing it by hand. /sigh

Matt Simmons
http://www.standalone-sysadmin.com

Written on 27 July 2009.
« The anatomy of a hack to get around try:/finally: and generators
Spammers are quite dedicated in their address scraping »

Page tools: View Source, View Normal, Add Comment.
Search:
Login: Password:
Atom Syndication: Recent Comments.

Last modified: Mon Jul 27 00:39:24 2009
This dinky wiki is brought to you by the Insane Hackers Guild, Python sub-branch.