Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Propose a +1 step (for self review) for the code reviews steps.

The usual steps for a code review are:

  1. Author creates a change.
  2. Author submits one or more patches to Gerrit.
  3. Author self-reviews the change following the "DOs and DON'Ts" below.
  4. Author ensures that all tests pass.
  5. Author adds reviewers and votes +1 on the change (indicating that the change is ready for review).
  6. Reviewer reviews the change and adds comments and/or votes +2 on the change.
  7. If the reviewer adds comments and does not vote +2, the author
    1. addresses each comment by either answering a question, changing the code, or explaining the code should not be changed and
    2. goes back to step 2 submitting the changed code and/or replies.
  8. If the reviewer votes +2, the author merges the change.

DOs and DON'Ts

Code reviews take a significant amount of time in terms of effort spent by the reviewer and the latency incurred by waiting on reviews.
By following a few simple rules we can improve the review experience and turnaround times:

...

  •  

    Is the code of the change formatted using HyracksFormattingProfile?

  •  

    Are xml files indented correctly? (using 4 spaces with no tabs)

  •  

    Are there any extra white spaces?

  •  

    Are member variables which are not intended to be changed made final?

  •  

    Is logging performed properly (i.e. using log4j with the name set as Class.getName())?

  •  

    Are class & member access as restricted as it can be?

  •  

    Are names of variables and methods meaningful and convey intention?

  •  

    Is logging performed properly (i.e. using log4j with the name set as Class.getName())?

 

...