-
Notifications
You must be signed in to change notification settings - Fork 115
Development Practices
Alex Seaton edited this page Dec 21, 2023
·
4 revisions
- 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.
- 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.
- 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 testing quality 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.
ArcticDB Wiki