DevTools/Code Review Checklist

From MozillaWiki
Jump to: navigation, search
Do not edit this page THIS PAGE IS PROPOSED FOR DELETION 26 Feb 2018
Moved to devtools docs (http://docs.firefox-dev.tools/contributing/code-reviews.html)

This checklist is primarily useful for DevTools reviewers as it lists all points potentially important to check while reviewing code, but it can serve as a set of guidelines for patch authors too.

Bug status and patch file

Manual testing

  • Verify the feature or fix
  • Report problems in the global review comment
  • Decide which bugs need to block landing the change, and which can be fixed as follow-ups instead

Automated testing

  • Run new/modified tests, with and without e10s
  • Watch out for tests that pass but log exceptions or end before protocol requests are handled
  • Watch out for slow/long tests: suggest many small tests rather than single long tests

Code review

Finalize the review

  • R+ : the code should land as soon as possible
  • R+ with comments: there are some comments, but they are minor enough, or don't require a new review once addressed, trust the author
  • R cancel / R- / F+: there is something wrong with the code, and a new review is required