DEV Community

Cover image for Code Smell 239 - Big Pull Request
Maxi Contieri
Maxi Contieri

Posted on • Originally published at maximilianocontieri.com

5

Code Smell 239 - Big Pull Request

You make too many different changes in a single pull request

TL;DR: Always stick to baby steps

Problems

  • Readability

  • Code Review time and complexity

  • Merge Conflicts

  • Testing Challenges

Solutions

  1. Break the change in atomic parts

Context

When pull requests become very large, they can pose several challenges and problems for development teams.

You must avoid merge requests making different unrelated changes.

Sample Code

Wrong

function generateFibonacci(ordinal) {
  const fibonacciSequence = [0, 1];

  for (let index = index; index < ordinal; index++) {
    const nextFibonacci = fibonacciSequence[index - 1]
          + fibonacciSequence[index - 2];
    fibonacciSequence.push(nextFibonacci);
  }

  return fibonacciSequence;
}

// This function solves a very different problem
// You should not mix them in a single pull request

function voyagerDistanceFromEarth(currentDistanceInKms, yearsTravelled) {
  const speedOfVoyagerInKmS = 17; 

  return currentDistanceInKms + 
        speedOfVoyagerInKmS * yearsTravelled * 60 * 60 * 24 * 365.25;
}
Enter fullscreen mode Exit fullscreen mode

Right

function generateFibonacci(ordinal) {
  const fibonacciSequence = [0, 1];

  for (let index = index; index < ordinal; index++) {
    const nextFibonacci = fibonacciSequence[index - 1]
          + fibonacciSequence[index - 2];
    fibonacciSequence.push(nextFibonacci);
  }

  return fibonacciSequence;
}

// You break it into two different pull requests
Enter fullscreen mode Exit fullscreen mode

Detection

[X] Automatic

You can put a threshold and a warning on big merge requests.

Exceptions

  • Big refactors that cannot be made with baby steps

Tags

  • Complexity

Level

[ X] Beginner

AI Assistants

AI assistants do not create pull requests.

They generate the code you need.

Conclusion

Software engineers must be experts at managing (and avoiding) accidental complexity.

Relations

Disclaimer

Code Smells are my opinion.

Credits

Photo by Håkon Grimstad on Unsplash


First make the change easy (warning: this might be hard), then make the easy change.

Kent Beck


This article is part of the CodeSmell Series.

Heroku

This site is built on Heroku

Join the ranks of developers at Salesforce, Airbase, DEV, and more who deploy their mission critical applications on Heroku. Sign up today and launch your first app!

Get Started

Top comments (0)

Billboard image

The Next Generation Developer Platform

Coherence is the first Platform-as-a-Service you can control. Unlike "black-box" platforms that are opinionated about the infra you can deploy, Coherence is powered by CNC, the open-source IaC framework, which offers limitless customization.

Learn more

👋 Kindness is contagious

Please leave a ❤️ or a friendly comment on this post if you found it helpful!

Okay