Example checklist for reviewing pull requests

• Read waffle story and all comments (also any related stories)
• Pull branch and start the rails server against that branch
• Exercise the changed code in the UI
• Run all rspec tests with external services blocked (all rspec tests should use stubs and not connect to external services)
  • In spec/spec_helper.rb, comment out this statement: WebMock.allow_net_connect!
• Review code changes in github
• Review code changes in context - in your editor; look for:
  • Consistency of style
  • Code that convoluted, unclear
  • Code that needs a comment 1) in order to prevent misunderstanding of the developer's intent, or 2) that implements business rule logic that is not obvious nor necessarily known by all developers.
  • Method return values should be explained in a comment if not fairly obvious. For example, this comment explains the return value of a method that would require significant code review to understand otherwise:

   def self.match_jobs(resume_id)
      # If match exists, returns an array of job matches.  Each match
      # is represented as an array with 2 values - the first value is
      # the job ID, and the second the match score (a float, with
      # one digit to right of decimal point).
      # Otherwise returns nil(in case of resume not found)
  
      # The array is sorted by score (descending)
  
      # Example return value:
      # [ [8, 4.7], [5, 3.3], [3, 2.1] ]
      # Job (ID) 8 matched resume with a score of 4.7, job 5 with 3.3, etc.
  

  • Code that would be better if refactored, e.g. into a helper method
  • Code that is unneeded because there is a utility/helper method available
• Confirm sufficient acceptance test coverage
• Confirm sufficient rspec test coverage