Pull Requests reviews with the team

Who says there has to be a meeting? 

My company has an organizational rule that is applied to all team's Definition of Done.  It states that all Pull Requests must have 2 reviews from people that were not involved in writing the code in the Pull Request.  This is done without meetings.  A developer will put a message in a specified Slack channel asking for code review.  If a there is a need for a specific developer or for a specific speciality to be involved, that is mentioned in the Slack message.  Developers will review the code, leaving comments in the source management system. When all comments/concerns have been addressed and 2 or more people approve, the Pull Request is moved forward and merged.  If the people involved feel like they need to talk/meet, they set it up.  But that only occurs on a small fraction of the reviews.  The vast majority of them are done without the need for any gathering of people. 

This approach was initiated by the entire development staff.  They saw a need and came up with a solution that was minimally invasive to their work. 

• Do the pull requests need to be merged before the meeting?
• Do you merge the PRs in the meeting?
• Do you discuss every PR?
• Do you scroll through all the code?

Suppose you didn't do that integration work and make those checks. Would you then have a product increment that is fit for immediate release? Would it be clear to those attending the review that the work being examined is "Done" and complete?

There are different ways to review pull requests and different criteria for choosing what method to use. It's not uncommon to require at least one reviewer on a PR. Sometimes, larger changes or changes to core/common/shared functionality may review additional reviewers. In most cases, even for these larger reviews, you don't necessarily need a meeting. Sometimes, a reviewer may ask for a meeting if there is a lot of back-and-forth conversation happening in comments on the PR. Other times, the person requesting the review may ask for a meeting to be able to walk the reviewers through a more complex change.

Most of your questions - merging in the meeting or after, if you need a meeting for every PR, if the meeting is a walkthrough of the code or just a chance to get people in the same room to discuss the code as they review it on their own - depends on the practices that the team chooses. The only thing that I would recommend is that you do not merge pull requests prior to the meeting if you are using the PR as a code review.

Ultimately, the best thing to do is have a conversation with the team and don't be afraid to experiment. If something isn't working, change it. Even if it is working, don't be afraid to try something new to see if its better. Scrum gives you plenty of opportunities to inspect your way of working, run experiments, and make changes.

Hi,

I am working in a company, where the number of minimum reviewers of a PR are dictated by the leader of our department. We are working in a scrum team, but we have no right of making this decision by ourself. It was just a decision made by the leader of our department.

We are working in a very small scrum team, and the situation is, that we sometimes cannot start a deployment, because we did not get enough reviewers to merge the branch in BitBucket. So this is a heavy impediment for our daily work.

We talked to our scrum master and also to our product owner, but both stated, that this is just a normal decision, which should not be made by the developer team.

Is that scrum compliant? Normally we should be self organized as we understood scrum!?

Thanks in advance for every helpful answers ;-)

Cheers

peter

Code review is not mentioned anywhere in the Scrum Guide.  It is not part of the Scrum framework.  However, best practices in software development usually include some kind of peer review of code for a variety of reasons.  And the review is usually mentioned as part of the Definition of Done (or has been in every organization I have been associated with). 

The Scrum Guide explains the Definition of Done in the section that describes the Increment as it is the commitment for an Increment. This is one statement made in that section:

If the Definition of Done for an increment is part of the standards of the organization, all Scrum Teams must follow it as a minimum. 

The situation you describe would fall under that statement in my opinion.  The organization (i.e. leader of the department) has created a condition for the organizational Definition of Done and the Scrum Team must follow it as a minimum. 

Scrum does have an expectation of self-organized, self-managing teams.  However, that doesn't guarantee that the team is allowed to do anything they want in any way that they choose.  Most teams are part of a larger organization.  Organizations usually have some rules and guidelines in which everyone must operate for the good of the organization.  

So this is a heavy impediment for our daily work.

In Scrum it can't be your work if you don't own it, because you are not then accountable.

What are the consequences for the higher-ups who seem to make the decisions, and do they have an incentive to improve matters?