SQLServerCentral Editorial

The Art of the Code Review

,

The inspiration for this was a piece about the art of the good code review. Throughout my career, I've seen code reviews grow and change. From formal meetings to automated notifications and asynchronous discussions to complete lip service to the process. I'd like to think that most organizations are beyond the latter and there is some sort of review beyond the developer, but I still see a lack of other eyes looking at code before it's deployed, especially database code.

The article above opens with the idea of why we review code. The main reason is to create ownership, or more specifically, shared ownership. I had never thought of it in these terms, even though I think the ideas of standards and patterns are certainly shared items. Having everyone take ownership not only keeps quality high but could help you share knowledge and also ensure everyone feels a responsibility to safeguard all the code. This also helps everyone keep an eye on the larger picture of the entire codebase.

I know lots of modern application developers are very familiar with pull requests, though I think these are still somewhat rare in the database world. This is a notification that someone would like their code to be put together with everyone else's code. A good code review does start with a good pull request, as the idea is to have enough information to let the reviewer decide if they should approve things. PRs should also be focused, so if you are making a major change in one object and refactoring another, make those two different PRs. That way I can reject one without the other.

Overall I like the suggestions in the post, but I worry about one of them. The author notes that if there are minor changes requested in a comment, you should pre-approve the PR and trust the author to address the issues. In today's very busy world, and with the challenge of changing code once it's deployed to a database, I don't know if I'd follow this for SQL code. Maybe for C#, but if I have to live with your code for a decade, I don't want mistakes deployed that could be prevented.

Rate

5 (2)

You rated this post out of 5. Change rating

Share

Share

Rate

5 (2)

You rated this post out of 5. Change rating