Improve the efficiency of code review process within 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 within the team.
Guidelines
- Well Groomed JIRA Tickets:
- Code review process can be faster if a ticket is properly groomed. Very rarely someone will start a discussion on the PR' which could delay the code review.
- To avoid re-do, discuss major refactoring solution within the team or anyone on board before implementing it.
- Need of F.I.F.R. within team:
- The Initial Fast Internal Review (F.I.F.R.) is critical within the team.
- If you really need to tag someone in Cambridge, make sure your PR has an an F.I.F.R.
- Why? It's a 12hr turnaround, between Cambridge and Lahore, and you would be ready to merge as soon as your 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, Share the responsibility by referring someone else in 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 definatly 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).
- 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.
- Use of /wiki/spaces/TNL/pages/38863270 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.
- Start the communication if do not know where to start. Following ways can help you.
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
USED AND CHANGED THE BLOW AND ADDED ABOVE.
PLEASE UPDATE ACCORDINGLY AND DELETE THE FOLLOWING.
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
- Start the communication if do not know where to start. Following ways can help you.
- HipChat: 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?
- Follow the traditional pattern of tagging someone on PR
- Code review process can be faster if the ticket is properly groomed because sometimes discussion starts on the PR' and it delays review.
- Major refactoring of solution provided for a particular issue might be avoided by discussing solution with teammates or anyone on board before implementing it.
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 is busy and can't find time for review, he or she can refer for review to someone else. We may have another column(tag) for an issue named something like
- 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.
- 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.