Pull Requests and Code Reviews

Share on facebook
Share on twitter
Share on linkedin
Share on reddit

Run-through

The value of AI-powered Business Intelligence

What benefits can AI-powered Business Intelligence can bring? While this may be of interest to true blue BI Managers and Analysts, business intelligence is making its way into a lot of other roles. Between skill shortages, job title overlap, and a growing interest in cultivating cross-functional teams, BI is pretty much for everyone who has a mind to be more efficient, productive, and profitable.

Read More »

The Differences of DevOps and DataOps

How different are DevOps and DataOps? Are we splitting hairs or hairstyles? Both are aligned to Agile principles and have a number of processes in common, but they have some key differences, too. Both are also very complex and aim to solve mostly different sets of challenges while also working to improve team cross-functionality. So, let’s take a look to get a better understanding of both.

Read More »

There are two words that will help open a lot of doors in life. Push and Pull. Here, we’re going to talk about Pull Requests. GitHub introduced the term Pull Request to the software development world in 2008, starting on the very first day GitHub launched. Pull requests have since become a standard part of the software development process and are closely associated with code reviews. Code reviews involve teamwork and collaboration on code and serve as one of the four main drivers of software development, alongside speed, quality, and efficiency.

What is a Pull Request in GitHub?

A pull request is a notification that a developer has made code changes locally that need to be reviewed, and if good, merged with the master branch. With some git branching strategies, the merge is with a feature, develop, or release branch. And, in Continuous Integration and Continuous Delivery/Deployment, pull requests are automated, though code reviews are still conducted.

What’s the Difference Between a Pull Request and a Push?

The difference is actually pretty simple:

  • A push equates to assigning a task to a developer who must then start by making a clone of the “main branch” where changes can be freely done.
  • A pull takes the recent changes or commits in a developer’s local or cloned branch and, if they pass review and testing, merges them with your main branch.

How GitHub Pull Requests Fit the Development Process

The image above may help show where pull requests fit into a six-step development process:

  1. Create a Branch – This is the push. The actual branch you’re pushing depends on your team’s branching strategy. This should always be the master branch in GitHub Flow and Trunk Based Development – It could be different (Feature, Develop, Release) if you use Git Flow, One Flow or GitLab Flow.
  2. Add Commits – Commits are the files and changes to files that you make. Each commit should have a clear description of the reason for the commit. Each commit is a distinct change applied to your local branch first.
  3. Open a Pull Request – This is a formal request for your commit to be reviewed by another developer.
  4. Code Review – A developer agrees to perform a code review on your commit, which may be done formally or informally. If approved, your commit takes a step forward (to #5 or #6 depending on your team’s process). If not approved, you return to #2 with comments advising you of issues that need to be fixed.
  5. Deploy – If your commit passes the code review, most teams will deploy it for further, more complex testing in a simulated production environment. If the changes don’t pass tests, you return to #2 with feedback from the test.
  6. Merge – Finally, after your code is reviewed, approved, and passes all tests, your commit is merged with the master branch – which, in GitHub Flow and TBD should always be deployment-ready.

What Happens After a Pull Request is Created?

After a developer creates a pull request, notifications are sent to the email addresses you’ve defined under GitHub Settings > Notifications. Besides individual email addresses, you can also point emails to a mailing list.

When a developer is free, they’ll determine whether a formal or informal code review is required. The outcome of their review can have three possible results:

  1. Accept – the reviewer deems the code/changes to be in good order and approves merging them.
  2. Reject – the reviewer finds issues and identifies them to the author to fix – and resubmit.
  3. Comment – observations are shared but a decision is deferred to another developer with more expertise.

Does Every Pull Request Involve a Code Review?

Yes, but… remember there are several types of code review. Code reviews for pull requests can be (formal) technical reviews or (informal) inspections. Additionally, in Continuous Integration, Delivery, and Deployment, pull requests are automated though code reviews are still done but usually at a later stage.

In formal code reviews, the author and reviewer sit down together to discuss the changes and make revisions, as needed. Formal code reviews are best for when the PR involves significant amounts of code, code with many changes, or very complex code. A general rule of thumb is that a formal code review can cover 400 lines of code in an hour and catch 70-90% of defects.
Informal reviews can be handled via emails or comments, and are best for handling many small changes in code.

Coding Standards and Linters

Enforcing coding standards can be done with a code linter – a program that can analyze your code to help force compliance to coding styles. Not everyone likes code linters, and there are some like Adam Nathaniel Davis who used to hate them but have come to love them. He makes some good points to underscore the difference between Code Nazism and Good Enough Code. Suffice that if it’s defined in your style guide, it’s worth configuring in your linter. Check out this awesome list of linters if you’re in need of one.

Same Criteria for Writing and Reviewing Code

The same principles are involved in writing high-quality code and reviewing it. Developers are involved and frequently switch between writing and reviewing code. It’s simply the “Golden Rule” for developers to review their own work according to the same checklist and standards by which it will be reviewed by their teammates after the pull request is created.

Two heads are better than one, suffice that a fresh pair of eyes can more easily find certain types of defects and issues than the original author can. The brain’s a funny thing when it comes to how it approaches high-level tasks and minute details. There’s a lot to be considered when writing and reviewing code, including but not limited to:

  • Does the code comply with your team’s coding standards?
  • Is the code modular, logically structured and easy to read?
  • Are variables and functions named suitably and meaningfully?
  • Are functions, methods, behaviors, etc. sufficiently documented?
  • Does the code work?
  • How complex is the code and can it be simplified or is it “good enough”?
  • Is there any duplicate or unnecessary code?
  • How much of the code is covered with meaningful tests?
  • Are there any “untested hot spots” that should be scrutinized in greater detail?Does the code cover all relevant failure states?
  • Does the code meet performance requirements or have performance issues?

Pull Request Analysis with the Gitential App

A variety of metrics are involved in tracking team collaboration, like responsiveness, co-authoring, review coverage, ration of unreviewed pull requests. In Software Development Metrics: The Sum Of Developer Skill And Team Organization we discussed at some length how team size and structure, along with the ratio of senior to mid-level and junior developers can introduce inefficiency. How long it takes for a pull request to be reviewed often adds a lot of inefficiency.

All developers should be involved in code reviews – and in reviewing pull requests as it’s an effective way for all developers to learn. One component of this is checking to see who your developers are collaborating with.

For example, we have a new developer who is only collaborating with one other developer – who also happens to be a new developer. Letting this continue will only hold both developers back and probably have a broader, negative impact across all of their performance metrics.

At the other extreme, if both developers were senior, experienced developers – without switching things up a bit, a) your team is developing knowledge silos, and b) the rest of the team is being held back in not being able to learn from some of your best developers. The follow-on impact is that if either of these developers leaves, it will have a pronounced negative impact on your team in supporting the portion of work they specialized in.

Teams View

With Gitential’s Teams view you can easily overview all teams created in the workspace, and or just check a singular team summary and have more a compact or extended view.

Want More on Pull Requests?

There’s a lot more to pull requests depending on which repository management system you use and – your own work processes. If you’d like more on pull requests, we’re happy to recommend:

About Gitential

Our team at Gitential constantly watches trends across the software development industry. We also get down into the code with you to find ways to make it easier for you to make your team more efficient. Please take a moment to arrange a free demo – or sign up for a free trial, no credit card is needed.

Did you like our content?

Spread the word

Share on facebook
Share on twitter
Share on linkedin
Share on reddit

Subscribe to Our Newsletter

Don't miss our latest updates.
All About Software Engineering Best Practices, Productivity Measurement, Performance Analytics, Software Team Management and more.

Did you like our content?

Spread the word

Share on facebook
Share on twitter
Share on linkedin
Share on reddit

Subscribe to Our Newsletter

Don't miss our latest updates. All About Software Engineering Best Practices, Productivity Measurement, Performance Analytics, Software Team Management and more.