There are a lot of resources out there that are checklists for reviewing your own code, before sending it for review, such as this one.

Here is another! It is split in to sections.

As a priority though, before getting code free of tech debt and perfect, one should deliver and deploy the business value first if needed, then right after refactor as needed.

Straight Forward Checks

For future proofing:

  • does this work fit in with the future tech direction
  • enable new features of the language if relevant
  • ask if the best libraries for the use case used

The basics which need a bit of thought:

  • when passing an object in code - it is good to pass the whole one, as signature then won't know about granular workings, and also if is changed in future to use other properties in it, signature won't change - but case by case basis, you can also pass minimal info in
  • is the code well tested? Not just are there tests at all, but are the tests ones which would need to be re-written if the implementation changed? Is there a more future-proof way to test this?
  • test at business level, abstract only when need to, avoid mocking if appropriate

The basics most of which can be or are automated:

  • anything that would be a intellisense suggestion
  • follow best practices
  • formatting
  • add documentation to methods, even architecture decision records

Feature Specific Things:

Have all of these been considered:

  • any service should be a  “technical authority for a business capability"
  • domain driven design should be used if appropriate (but don't use things without reasons or because they are buzzwords!), with bounded contexts mapped out
  • when naming tests - name as per business, not as per data transfer objects or random properties in the code
  • Things that change together live together

Simplicity/Clarity:

  • in the pull request description, have you provided a reasonable amount of detail? a good structure is to state functional changes (actual changes to the user) and structural changes (no changes to the user, structural code changes only such as refactoring)
  • is the pull request too large
  • would this code be confusing to a new hire? Is there a way to make it simpler or easier to understand?
  • does prep work need to be done eg refactoring?
  • is there a better way to structure this code? Is it following best practices?
  • is the code grouped in a way that makes sense? Is the data stored in the best way? e.g. including timezone info for datetimes, using a structured format (as logging fields, or db files, json fields) instead of encoding as (unstructured) string.
  • is something being written from scratch when there is a library that would solve this?

For When You are The Reviewer

And lastly, obviously, it's always best to check out the code in an IDE when reviewing others' pull requests.