A code review checklist

twelve points on one screen

What it is

The list you read any generated piece of code against before it goes into the project. The details are in chapters 2 and 4; here there is only the order.

First, three questions

1. Do I understand what every line does? If not, don't paste it — ask (article 2.1).
2. Does this solve my problem or a similar one? The commonest substitution: the problem the AI understood has been solved, not the one you set (article 3.1).
3. What happens if this breaks? The answer decides how much time to spend on the remaining points (article 1.5).

The twelve points

1. Errors. Is there handling where something could go wrong: the network, a file, parsing data, division. An empty except is a red flag.

2. Edge cases. An empty list, None, zero, a negative number, a very long string, two identical values.

3. External data is validated. An API response, user input, a file's contents — none of it is parsed without a check (article 7.3).

4. Secrets. No keys, passwords or tokens in the code. Not "temporarily", not "as an example" (article 5.7).

5. New dependencies. A library appeared that nobody asked for? Ask why. Often it can be left out.

6. The edit zone. Only what was asked for has been changed. Pieces rewritten "while we were there" are grounds for reverting and asking again (article 3.13).

7. Files and paths. Relative paths to data, hard-wired addresses and /home/user/... in the code (articles 12.1, 13.4).

8. Repetition. The same action has appeared in a third place — time to extract it.

9. Names. The names say what's inside. data2, temp_fix, handle usually mean the author didn't know either.

10. Layer dependencies. A database query from a screen's handler, an import pointing upwards (article 13.1).

11. What was deleted. In the diff, look not only at what was added but at the minuses: what disappeared along with the edit (article 2.13).

12. The checks are intact. Weren't the linter, the types or the tests switched off just to make it "pass" (article 2.14)?

Then, run it

Reading doesn't replace running, and running doesn't replace reading. The minimum:

• run it and walk through your scenario by hand;
• walk through the unhappy scenario: turn off the network, pass an empty value;
• run the tests if there are any, and read what they actually check (article 4.6);
• look at the whole of the changed files in git diff before committing.