Development Practices

ArcticDB Development Practices

Pull Requests

• We only require one reviewer.
  • If you are asked to review a change, but do not feel qualified to do so, then inform the author. Do not approve code in areas you do not understand.
  • If your change is risky or in a cross-cutting area you should consider asking two reviewers to review it.
• For non-trivial code review comments, only the person making the PR comment should mark it as resolved. They should check that the comment has been addressed satisfactorily before doing so. Anyone can mark comments as resolved if the comment author has replied that they're happy with the current state.
• The PR author should merge their own PR, usually as soon as possible after it is approved and its CI has passed.
• Our CI only runs automatically against PRs. You can help reduce our compute costs by only opening PRs when you want feedback from the rest of the team.
• You should message your desired code reviewer when your PR is ready, rather than relying on them to monitor Github.
• You should also message the reviewer after addressing review comments such that the PR is ready for another review, for the same reason.
• Reviewers should start their review within one working day of being asked. Reviewers are not expected to context switch and immediately review.
• Pay particular attention to the quality of testing when reviewing a PR, consider test cases that the author may have missed. For complex changes, consider checking out the code and stepping through it.
• We aim to work iteratively and raise frequent, small PRs. A feature does not need to be complete before code for it can be merged, but master should always be releasable.