Review code effectively using quickfix
From Vim Tips Wiki
created 2004 · complexity basic · author Karthick Gururaj · version 5.7
If you do code reviews on a regular basis, this tip could be of help to you. This tip is about:
- Helping the reviewer review code
- Helping the author in collating reviews from various peers
- Helping the author in fixing the review comments
First learn about quickfix if you don't know about it already :help quickfix.
If you type your comments in a file in the format as shown below, then the author can use the quickfix mode for zeroing on the exact file/line.
<file_name>:<line_number>: <your comment in a single line>
For filling in the file name/line number automatically, put the following code in your vimrc:
function SavePosition() let g:file_name=expand("%:t") let g:line_number=line(".") let g:reviewer_initials="KG" " Your initials endfunction function InsertComment() execute "normal i". g:file_name . ":" . g:line_number . ": " . g:reviewer_initials . " - " startinsert endfunction nmap ,sp :call SavePosition()<CR> nmap ,ic :call InsertComment()<CR>
Typical review session:
- A reviewer open the code to review, positions the cursor on the line they want to comment on, and types ",sp"
- They then open a text file in the same Vim session (say my_comment.txt) and type ",ic" - this puts the file name and the line number
- The comment is typed next to the line number, all in a single line. This makes it possible for the file to be sorted later
- Send the comments to the author of the code
- The author collates the inputs from various reviewers into one file (by simply concatenating them) and sorts it. Now the comments are arranged per file, in the order of line numbers (in a file called say, all_comments.txt)
- Using the
:cfile all_comments.txtthe author can now navigate through all the comments.
This sounds like a techno overkill.
You cannot assume reviewers like nifty vim tip showoff instead of looking for serious issues, then again you never know who works in dot coms!
For some reason when I try and use this I get an error:
Invalid expression: "normal i". file_name . ":" . line_number . ": " .
You need the line_number, etc as global variables (see :help g:var). Also, you need to remove that trailing "." at the end of the line:
"normal i". file_name . ":" . line_number . ": "
Maybe it's better to expand the whole file name, so it's location will be viewed in the quickfix file, too.
That is true. I didn't read the help properly, and assumed that expand("%") gives the entire path name (which will be different for reviewer and the author). Instead, it puts the relative path. That works great if there are multiple files to be reviewed in a directory hierarchy. Thanks!
I have uploaded an expanded version of this tip to script#1041. The script now can handle multi-line comments and defect type classification.
What if the code is being reviewed by two or more persons? Are they required to explicitly specify their initials? According to your code it does seem that way. One more thing is before inserting a comment we have to say according to your code \ic, is this a built-in command or it can be changed to something else?
Yes, they'll have to specify their initials explicitly. And \ic is not a built-in command, it is a user defined map. See :help map for more information