CodeReviews

Responsibilities of Merge Proposal (MP) creator

  • Test the functionality before creating MP (you were actually running your branch in a real environment)
  • A test is added. The lower level the better (1. unit test, 2. autopilot test, 3. manual test). Manual tests are strongly discouraged and for visual stuff only (layout, rendering, ...).
  • If there is an autopilot test add Thomi (~thomir) or Chris (~veebers) as a reviewers. Ping them on IRC to make sure they see the review request!

  • All the bugs you fixed are linked to your branch
  • Commit message in the launchpad Merge Proposal is set
  • Merge proposal description is matching the following template:


= Problem description =

<describe the problem being fixed, or refer to linked bugs>

= The fix =

<describe how the code changes in the diff fix the problem described>

= Test coverage =

<What tests confirm correct behaviour>


  • Ubuntu processes are followed. This primarily means you are aware of:
  • Design review is requested when it is needed. It is needed when either
    • A visual change has been made to the interface and/or
    • A change has been made to the user facing functionality
  • If a packaging change is needed add integration team as a reviewer -- either ~timo-jyrinki (mirv) or ~sil2100

Responsibilities of reviewer

  • Coding style
  • Code correctness
    • flaws in the code
    • performance
    • ...
  • Does the merge proposal contain unrelated changes?
  • Check if test was added. The lower level the better (1. unit test, 2. autopilot test, 3. manual test). Manual tests are strongly discouraged and for visual stuff only (layout, rendering, ...).
  • Make sure it builds and tests are passing (ideally just check jenkins results; jenkins votes on the MP automatically) -- verify 'make check' passes if available, since jenkins does not always run these tests
  • Check if it was tested by MP creator (check the description)
  • Check if there is an autopilot test and if Thomi (~thomir) or Chris (~veebers) were added as reviewers

  • Check if design review is requested when it is needed. It is needed when either
    • A visual change has been made to the interface and/or
    • A change has been made to the user facing functionality
  • Check for a packaging change and whether ~timo-jyrinki (mirv) or ~sil2100 were added as reviewers

  • Check if all the bugs are linked to this branch
  • Make sure ubuntu processes are followed. This primarily means you are aware of:
  • Check commit message
  • Don't forget about the global state of the MP if you are the last reviewer

Unity/CodeReviews (last edited 2013-01-22 15:09:48 by 198)