Page Comparison

Improve the efficiency of code review process within team

...

Escalation Team. This document will provide a set of guidelines which can be followed according to the scenario which can help efficient feedback from the reviewers. 

Problem statement

• Escalation covers multiple teams (TNL, PLAT, ECOM) and sometimes it's unclear whom to ask for a code review :
We can hipchat the PR in a specific room related to the domain and ask someone, who is interested in this PR to review.If someone does not take initiative?then we follow the traditional pattern of tagging someone on PR
• within the team. 

Guidelines

• Well Groomed JIRA Tickets:
  • Code review process can be faster if
  the
  • a ticket is properly groomed
  because sometimes discussion starts
  • . Very rarely someone will start a discussion on the PR'
  and it delays
  • which could delay the code review.
  • Major refactoring of solution provided for a particular issue might be avoided by discussing solution with teammates or To avoid re-do, discuss major refactoring solution within the team or anyone on board before implementing it.
• Need of F.I.R. within team:
  
  • The Fast Internal Review (F.I.R.) is critical within the team.
  • If you really need to tag someone in Cambridge, make sure your PR has an F.I.R.
    • Why? It's a 12hr turnaround, between Cambridge and Lahore
  teams, Lahore team could tag people among ourselves to get at least one initial review started.
• Encourage reviews to be done within Lahore team.
• If someone
  • • , and you would be ready to merge as soon as you get the 2nd review.
• Taking and Sharing the Responsibility:
  • Code review is as important as fixing issues. 
  • Its reviewers' responsibility to ask if he is unclear about anything in the PR.
  • If a reviewer is busy and can't find time for a review,
he or she can refer for review to someone else.
• We may have another column(tag) for an issue named something like  'Review Needed' in JIRA board, so anyone can pick it up and review it.
• Mostly, people are busy with their sprints, we could circulate an email to the engineering department to discuss if tagging their team members is an option?.
• We could identify clearly who are the reviewers and other people who might be interested in a particular PR. If the PR gets accepted by identified reviewers, we could just merge it and not wait for others if  have not received any concern from them.
• When we tag someone for review, we could add the timeline for the reviewers, if they have time they can review it otherwise they can simply excuse.
• We should follow the PR template /wiki/spaces/TNL/pages/38863270:
  Sometimes reviewers don't know the details and also request for Sandbox. If not given details or have created sandbox on PR creation; it would delay review cycle.
  • Share the responsibility by referring someone else to the team for the review.
• Idea of M.I.C:
  • The idea of Making It Count (M.I.C.) means the fix should go out. This will help us get improved velocity in Review Meeting. The number of merged issues will definitely improve the velocity of the team.
• The Sandbox Logic:
  • A sandbox is a must for every TNL issue and wherever it is applicable. It helps reviewer to observe where the problem was, and if it actually fixed the issue.
    • Sandbox Vs Stage theory: It can help quickly understand a fix. Stage would be experiencing the problem but not the Sandbox (as Sandbox would be pointing your branch).
• Use of Sustaining Pull Request Template:
  • The Pull Request Template covers all the major areas that a reviewer can think of. Using the template would ease a review cycle.
• Communication:
  
  • Start the communication if do not know where to start. Following ways can help you.
    
    • HipChat: Use `@team` in Escalation Group to ask for a review.
    • Skype: Send personal message a reviewer.
    • In-person: You can always jump to the person after asking his availability.