Code review guidelines
A good pull request:
- Does about one thing (new feature, bug fix, etc)
- Adds tests and documentation for any new functionality
- When fixing a bug, adds or fixes test that would have caught said bug
- Are the style guidelines being followed?
- Do the signatures make sense? Are they minimal and reusable?
- Does anything need to be functored over?
- Are there any error cases that aren't handled correctly?
- Are calls to
_exn functions justified? Are their preconditions for not
throwing an exception met? Is the exception it throws useful?
- There shouldn't be commented out code.
- No stray debug code lying around.
- Any logging is appropriate. All
Logger.trace logs should be inessential,
because they won't be shown to anyone by default.
- Should this code live in its library? Should it live in a different library?
- Does the code confuse you? Maybe there should be a comment, or it should be
- Does a behavior change break assumptions other code makes?