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
- 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;
}
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
Detection
- 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
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
Code Smell 170 - Refactor with Functional Changes
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
Software Engineering Great Quotes
This article is part of the CodeSmell Series.