Entry tags:
Code review
I recently mentioned work I am doing on relational databases. This reminds me of a code review issue. At work when I develop Java code for the OMERO server or suchlike I can often add some integration tests that automatically verify from the client's point of view that the server's behavior is indeed as desired. At present we don't have as much testing for our SQL scripts: if the script runs error-free and the server still seems to work then that's the main thing. I have been thinking about this lately and today for some of my trickier PL/pgSQL I was able to write some accompanying unit tests in SQL using a temporary table where each row records test inputs and outputs and if the test passed. This may surprise the eventual reviewer.
This reminded me of another issue that occasionally bothers me at work: I don't know how reliably careful code review occurs. People open a pull request on GitHub describing their work and, ideally, how to test it. (They
Furthermore, without clear testing instructions people are reluctant to touch others' pull requests. But, I don't want to make the testing plan too simple and clear because if I just list steps to follow and what to watch out for then things may not go as I expect, but probably the reviewer will just do what I already did when deciding my work was good enough. I would instead like them to understand enough about the pull request to guess a bit about how to test it in case they think of or try something that I didn't. For now, the reasonable compromise might be to give clear testing instructions but also to comment more generally on what the work might have affected.
This reminded me of another issue that occasionally bothers me at work: I don't know how reliably careful code review occurs. People open a pull request on GitHub describing their work and, ideally, how to test it. (They
requestthat we
pulltheir work into a main code branch.) Earlier this year GitHub added the feature of allowing a pull request template to be defined for people to edit in describing their work, to remind them what to include. Often the testing instructions are functional, testing how the application behaves, which is all very well but the code is another matter, and for any particular piece of work there may be few of us able to judge if the behavior was implemented in a good way.
Furthermore, without clear testing instructions people are reluctant to touch others' pull requests. But, I don't want to make the testing plan too simple and clear because if I just list steps to follow and what to watch out for then things may not go as I expect, but probably the reviewer will just do what I already did when deciding my work was good enough. I would instead like them to understand enough about the pull request to guess a bit about how to test it in case they think of or try something that I didn't. For now, the reasonable compromise might be to give clear testing instructions but also to comment more generally on what the work might have affected.