What are some things you keep an eye out for when reviewing pull requests? These are some of mine:
- Does this code comply with the team’s selected architecture?
- Clean coding practices
- Is this function too long, or does it do too many things? (Generally no more than 15-20 lines)
- Does this function or variable belong in this class?
- Would this naming make sense to the next reader?
- Is this value being accessed safely, especially if it were null?
- Could this array/dictionary access cause an indexing crash?
- Could this cause a potential data race with unexpected value changes?
- Does the algorithm perform efficiently?
- Is this code written idiomatically with the language?
- Is this code unit tested? If not, should it be?
Top comments (0)