1. 4

  2. 2

    A practical tip for code reviews: Develop a check list.

    For example, my check list for reviewing pull requests to the D standard library contains items like:

    • Documented in the Changelog
    • Code works without garbage collector
    • Uses new documentation format (backticks)
    • Sufficient asserts

    These are not hard rules, because then we could check them automatically. Not everything has to be documented in the Changelog. Not everything must work without the GC. Still, a reviewer should consider it. Having a check list prevents me from forgetting that.

    Developing a good checklist is not that easy and never finished. It should be as short as possible. Each item should be quick and easy to check (the “assert” example above fails this).

    1. 2

      For OCaml I made “must be in the Changelog” part of the continuous integration testing; our Travis checks fail if the Changelog file is not touched by a PR, unless the magic words “(no change entry required)” occur in one of the commit messages. I find this fairly useful, but some people dislike it. (But they dislike it the most in case where it flags their PRs, and in general it’s actually a good thing.) I think Changelog are super important and (for the OCaml project) we still have some progress to do on how to handle them.