Tiny Code

Tiny Code

Sunday, March 1, 2015

Making code reviews easier

Our source trees at work contain quite a few files spread out over many directories.  So reviewing code changes done by other engineers used to present a challenge.  Being handed 10 files to review used to mean manually finding each in the source tree so I could set up my diff tool with the appropriate paths to see what changes had been made to each file.  So I came up with a small bash script to automate the finding of base source files for comparison.

When I receive an e-mail containing source files to review, I copy all the modified source files to a directory called /c/review/new (this directory looks odd because I'm stuck on a Windows machine and run Cygwin to make it easier to use).  Then I cd to the top of a source tree with no changes I keep synched up with our SVN server and I type the name of my script below.  I call it "revcp" for review copy.  It finds and copies files from the current directory into a directory called /c/review/old.  Once that's done, I fire up WinMerge pointing at the old and new directories.

The only thing which can present a problem is that sometimes there are files for review for which there are multiple matches.  Makefiles are a prime example since we have quite a few of them.  The script will find all matches and prompt the user for which file to copy.

Is it perfect?  Not by any stretch of the imagination.  But for a "quick and dirty" tool which only took 15 or 20 minutes to write, it saves me quite a bit of time.  Now I no longer dread the arrival of code review requests.

#!/bin/bash
#
#   revcp (review copy)
#
# Description:
#   Given a single directory containing a group of source files to be
#   reviewed, this script will find matching files from the source tree
#   starting at the current directory for comparison.
#
# One time preparation:
#   1)  Modify NEW_DIR and OLD_DIR environment variables below to specify
#       the directory where you've placed the files to be reviewed and
#       the directory where you'd like the corresponding base files
#       to be copied.
#
# To use:
#   1)  Copy all files to be reviewed into the directory pointed to by the
#       environment variable NEW_DIR set below
#   2)  cd to a directory in your source tree above all files being reviewed
#       such as /c/svnBase/riot/client (where svnBase is the directory where
#       your svn source tree is checked out).
#   3)  Run this shell script under Cygwin or another Unix shell.
#   4)  If duplicate files are found by script the user will be prompted to
#       choose the desired file.  Once chosen, the script will manually copy
#       the selected file to OLD_DIR.
#   5)  Use WinMerge or another diff utility which can diff entire directories
#       to compare the contents of OLD_DIR with NEW_DIR
#

# This directory contains new files to review
NEW_DIR=/c/review/new

# Files found from current path will be copied to this directory
OLD_DIR=/c/review/old

FILES=$NEW_DIR/*

function FindMatchingFile
{
    CurFileName=`basename "$1"`
    NumFiles=`find . -name $CurFileName -print | wc -l`
    if [ $NumFiles -eq 1 ]; then
        echo "Copying $CurFileName"
        find . -name $CurFileName -exec cp {} $OLD_DIR \;
    elif [ $NumFiles -eq 0 ]; then
        echo "File $CurFileName not found... must be new file"
    else
        echo ""
        echo "Found multiple matches for file: $CurFileName"
        echo ""
        MatchingFiles=`find . -name $CurFileName -print`
        i=1
        for m in $MatchingFiles
        do
            echo "$i - $m"
            let "i=i+1"
        done
        echo ""
        echo -n "Enter number of the file to copy or 0 to skip: "
        read FileNum
        if [ $FileNum -eq 0 ]; then
            echo "Skipping $CurFileName"
        elif [ $FileNum -le $NumFiles ]; then
            i=1
            for m in $MatchingFiles
            do
                if [ $i -eq $FileNum ]; then
                    echo "Copying file #$i - $m"
                    cp $m $OLD_DIR
                fi
                let "i=i+1"
            done
        else
            echo "User input too large"
        fi
        echo ""
    fi
}

echo "revcp - copy files for review"
for f in $FILES
do
    FileNameToFind=`basename "$f"`
    FindMatchingFile $FileNameToFind
done

No comments: