You’ve spent hours coding that new feature. You’ve tested it locally. Everything works perfectly. You create a pull request with confidence, only to have it sent back with a long list of comments from a senior developer. Sound familiar?

Getting your code rejected repeatedly can be frustrating and demoralizing, especially when you’re just starting your programming career. But understanding why senior developers reject pull requests can help you grow as a developer and improve your code quality.

In this article, we’ll explore the common reasons why senior developers might reject your pull requests and provide actionable advice on how to address these issues. By the end, you’ll have a better understanding of what senior developers are looking for and how to increase the chances of your pull requests being accepted.

Table of Contents

Understanding the Senior Developer Mindset

Before diving into specific reasons for pull request rejections, it’s important to understand the perspective of senior developers. They aren’t rejecting your code to be mean or to assert dominance. They’re evaluating your code based on years of experience dealing with production systems, maintenance nightmares, and technical debt.

Senior developers typically look at code with these considerations in mind:

When your pull request gets rejected, it’s usually because it falls short in one or more of these areas. Now, let’s examine the specific issues that might be causing your pull requests to be rejected.

Code Quality Issues

Not Following Coding Standards

One of the most common reasons for pull request rejections is failing to adhere to the team’s coding standards. Every team has its own set of conventions regarding:

Many teams use linters and formatters like ESLint, Prettier, Black, or RuboCop to enforce these standards automatically. If your team uses these tools, make sure to run them before submitting your pull request.

For example, if your team follows a JavaScript standard that requires semicolons, but you submit code without them:

// Your submission
const getUserData = () => {
  const userId = getCurrentUser()
  return fetchUserDetails(userId)
}

// What the team expects
const getUserData = () => {
  const userId = getCurrentUser();
  return fetchUserDetails(userId);
};

This might seem trivial, but consistency is important for readability and maintenance.

Poor Variable and Function Naming

Clear, descriptive names are crucial for code readability. Senior developers often reject code with vague or misleading names.

Consider these examples:

// Poor naming
function process(d) {
  let x = d.filter(i => i.s === 'active');
  return x.map(i => i.v);
}

// Better naming
function processActiveItems(items) {
  let activeItems = items.filter(item => item.status === 'active');
  return activeItems.map(item => item.value);
}

The second version is much clearer about what the function does and what each variable represents.

Overly Complex Code

Senior developers value simplicity. If you’re trying to show off your programming prowess with clever tricks or unnecessarily complex solutions, your pull request might get rejected.

// Overly complex
const isEven = num => !Boolean(num & 1);

// Simpler and more readable
const isEven = num => num % 2 === 0;

Remember, the best code is often the simplest code that accomplishes the task effectively.

Duplicate Code

The DRY principle (Don’t Repeat Yourself) is a fundamental concept in software development. If your pull request contains duplicated logic that already exists elsewhere in the codebase, it will likely be rejected.

// Duplicate validation logic in multiple functions
function validateUserEmail(email) {
  const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
  return emailRegex.test(email);
}

function processUserRegistration(userData) {
  // Same regex duplicated here
  const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
  if (!emailRegex.test(userData.email)) {
    return { success: false, message: 'Invalid email' };
  }
  // Rest of the function
}

Instead, you should refactor common logic into shared functions or utilities.

Magic Numbers and Strings

Using “magic” numbers or strings (hardcoded values without explanation) makes code harder to understand and maintain.

// With magic numbers
if (user.age > 18) {
  allowAccess();
}

// Better approach
const MINIMUM_AGE_REQUIREMENT = 18;
if (user.age > MINIMUM_AGE_REQUIREMENT) {
  allowAccess();
}

The second approach makes it clear what the number 18 represents and allows for easier changes if the requirement changes.

Architectural and Design Concerns

Not Following Design Patterns

Most established codebases follow certain design patterns. If your code introduces a different pattern without good reason, it might be rejected.

For example, if your team uses the repository pattern for data access, but you directly use database queries in your service layer, your pull request might be rejected because it violates the established architecture.

Poor Separation of Concerns

Code should be organized so that each component has a single responsibility. If your code mixes concerns (like combining business logic with UI rendering), it will likely be rejected.

// Poor separation of concerns
function UserComponent() {
  async function handleSubmit() {
    // Data validation
    if (!username || username.length < 3) {
      setError('Username too short');
      return;
    }
    
    // API call directly in component
    const response = await fetch('/api/users', {
      method: 'POST',
      body: JSON.stringify({ username }),
    });
    
    // Response handling
    const data = await response.json();
    if (data.success) {
      // Database update logic
      localStorage.setItem('user', JSON.stringify(data.user));
      // UI state management
      setUser(data.user);
      navigate('/dashboard');
    }
  }
  
  // Component rendering...
}

A better approach would separate these concerns into different functions or even different files/modules.

Tight Coupling

Tightly coupled code is difficult to test and maintain. Senior developers prefer loosely coupled components that can be tested and modified independently.

// Tightly coupled
class UserService {
  constructor() {
    this.database = new PostgresDatabase();
  }
  
  getUser(id) {
    return this.database.query('SELECT * FROM users WHERE id = $1', [id]);
  }
}

// Loosely coupled with dependency injection
class UserService {
  constructor(database) {
    this.database = database;
  }
  
  getUser(id) {
    return this.database.query('SELECT * FROM users WHERE id = $1', [id]);
  }
}

The second approach allows for easier testing and more flexibility.

Reinventing the Wheel

If you’re implementing functionality that already exists in a standard library or a dependency that the project already uses, your pull request might be rejected.

// Reinventing array flattening
function flattenArray(arr) {
  let result = [];
  for (let i = 0; i < arr.length; i++) {
    if (Array.isArray(arr[i])) {
      result = result.concat(flattenArray(arr[i]));
    } else {
      result.push(arr[i]);
    }
  }
  return result;
}

// Using built-in method
const flattened = arr.flat(Infinity);

Always check if the functionality you need already exists before implementing it yourself.

Documentation and Communication Issues

Insufficient or Unclear Pull Request Description

Your pull request description is your chance to explain what you’ve done and why. If it’s vague or missing entirely, reviewers might reject your PR simply because they don’t understand its purpose.

A good pull request description should include:

Missing or Poor Code Comments

While clean, self-documenting code is ideal, complex logic often needs explanation. Senior developers might reject code that lacks necessary comments, especially for non-obvious implementations.

// Missing explanation for a complex algorithm
function calculateOptimalPath(nodes) {
  let visited = new Set();
  let pq = new PriorityQueue();
  pq.enqueue(startNode, 0);
  // ... 50 more lines of complex graph traversal logic
}

// Better with comments
function calculateOptimalPath(nodes) {
  // Using Dijkstra's algorithm to find the shortest path
  // between nodes in a weighted graph
  let visited = new Set();
  let pq = new PriorityQueue();
  pq.enqueue(startNode, 0);
  
  // Track visited nodes to avoid cycles
  // ... rest of the implementation with appropriate comments
}

Lack of Documentation Updates

If your changes affect existing functionality or APIs, you should update the relevant documentation. Failing to do so might result in your PR being rejected.

This includes:

Testing Problems

Insufficient Test Coverage

Many teams require new code to be covered by tests. If your pull request lacks tests or has inadequate test coverage, it might be rejected.

Good tests should cover:

// Example of a function with comprehensive tests
function divide(a, b) {
  if (b === 0) {
    throw new Error('Division by zero');
  }
  return a / b;
}

// Tests
test('divides numbers correctly', () => {
  expect(divide(10, 2)).toBe(5);
});

test('handles decimal results', () => {
  expect(divide(5, 2)).toBe(2.5);
});

test('throws error on division by zero', () => {
  expect(() => divide(10, 0)).toThrow('Division by zero');
});

Broken Tests

If your changes break existing tests, your pull request will almost certainly be rejected. Always run the full test suite before submitting a PR.

Testing Only the Happy Path

A common mistake is only testing the “happy path” and ignoring edge cases or error conditions.

// Only testing the happy path
function parseUserInput(input) {
  return JSON.parse(input);
}

// Test
test('parses valid JSON', () => {
  expect(parseUserInput('{"name":"John"}')).toEqual({name: 'John'});
});

// Missing tests for invalid JSON, which would throw an exception

Performance Concerns

Inefficient Algorithms

Senior developers are often concerned about algorithmic efficiency, especially for operations that might scale with data size or user load.

// Inefficient algorithm - O(n²) time complexity
function findDuplicates(array) {
  const duplicates = [];
  for (let i = 0; i < array.length; i++) {
    for (let j = i + 1; j < array.length; j++) {
      if (array[i] === array[j] && !duplicates.includes(array[i])) {
        duplicates.push(array[i]);
      }
    }
  }
  return duplicates;
}

// More efficient - O(n) time complexity
function findDuplicates(array) {
  const seen = new Set();
  const duplicates = new Set();
  
  for (const item of array) {
    if (seen.has(item)) {
      duplicates.add(item);
    } else {
      seen.add(item);
    }
  }
  
  return Array.from(duplicates);
}

Unnecessary Database Queries

Database operations are often bottlenecks in application performance. Code that makes excessive or inefficient database queries might be rejected.

// Inefficient - N+1 query problem
async function getUsersWithPosts() {
  const users = await db.query('SELECT * FROM users');
  
  // This makes a separate query for each user
  for (const user of users) {
    user.posts = await db.query('SELECT * FROM posts WHERE user_id = $1', [user.id]);
  }
  
  return users;
}

// More efficient - single join query
async function getUsersWithPosts() {
  return db.query(`
    SELECT users.*, posts.* 
    FROM users 
    LEFT JOIN posts ON users.id = posts.user_id
  `);
}

Memory Leaks

Code that doesn’t properly clean up resources or holds references to objects unnecessarily can cause memory leaks. Senior developers are likely to catch and reject such code.

// Potential memory leak in a React component
function LeakyComponent() {
  useEffect(() => {
    const interval = setInterval(() => {
      // Do something periodically
    }, 1000);
    
    // Missing cleanup function
  }, []);
  
  return <div>Component content</div>;
}

// Fixed version
function FixedComponent() {
  useEffect(() => {
    const interval = setInterval(() => {
      // Do something periodically
    }, 1000);
    
    // Proper cleanup
    return () => clearInterval(interval);
  }, []);
  
  return <div>Component content</div>;
}

Process Violations

Not Addressing Previous Feedback

If you’ve received feedback on a previous version of your pull request but haven’t addressed it in your update, your PR will likely be rejected again.

Always make sure to:

Too Many Changes in One Pull Request

Large pull requests are difficult to review and more likely to contain errors. Senior developers often prefer smaller, focused changes.

As a rule of thumb:

Merging Without Required Approvals

Many teams have policies requiring a certain number of approvals before a PR can be merged. Trying to bypass these requirements is a sure way to get your PR rejected.

Committing Directly to Protected Branches

Most teams protect certain branches (like main or master) and require changes to go through the pull request process. Attempting to commit directly to these branches will result in rejection.

What to Do Before Submitting a Pull Request

To increase the chances of your pull request being accepted, follow these steps before submitting:

1. Review Your Own Code First

Before asking others to review your code, review it yourself. Look for:

Many of these issues can be caught by linters and formatters, so run those tools before submitting.

2. Run All Tests

Make sure all existing tests pass and that your new code has appropriate test coverage. Don’t rely on CI to catch test failures; run the tests locally first.

3. Check Against the Team’s Pull Request Checklist

Many teams have a checklist for pull requests. If your team has one, go through it item by item before submitting.

A typical checklist might include:

4. Write a Clear Pull Request Description

As mentioned earlier, a good PR description helps reviewers understand your changes. Include:

5. Self-Review the Diff

Before submitting, look at the actual diff that will be included in your PR. This can help you catch unintended changes or files that shouldn’t be included.

Handling Feedback Constructively

Even with careful preparation, you’ll still receive feedback on your pull requests. How you handle this feedback can significantly impact your growth as a developer and your relationship with your team.

Don’t Take It Personally

Code reviews are about the code, not about you. When a senior developer points out issues in your code, they’re trying to help you improve, not criticize you personally.

Ask Questions

If you don’t understand a piece of feedback, ask for clarification. Most senior developers are happy to explain their reasoning and provide guidance.

// Instead of:
"I don't agree with this feedback."

// Try:
"I'm not sure I understand why we should use a Set here instead of an array. Could you explain the benefits in this context?"

Look for Patterns in Feedback

If you notice the same issues coming up repeatedly in feedback, focus on addressing those patterns in your future work. This shows that you’re learning and growing.

Express Gratitude

Thank reviewers for their time and feedback. Code review takes effort, and acknowledging that effort helps build positive relationships with your team members.

Use Feedback as a Learning Opportunity

Every piece of feedback is an opportunity to learn something new. When a senior developer suggests an alternative approach, take the time to understand why it’s better and how you can apply similar patterns in the future.

Conclusion

Having your pull requests rejected can be frustrating, but understanding why they’re being rejected is the first step toward improvement. Senior developers aren’t rejecting your code to be difficult; they’re trying to maintain code quality and help you grow as a developer.

By addressing the common issues we’ve discussed in this article, you can significantly increase the chances of your pull requests being accepted:

Remember that every rejected pull request is an opportunity to learn and improve. The feedback you receive today will make you a better developer tomorrow.

And who knows? With enough practice and improvement, you might find yourself on the other side of the review process, helping junior developers understand why their pull requests are being rejected and guiding them toward better coding practices.

Keep coding, keep learning, and don’t get discouraged by rejections. They’re just stepping stones on your path to becoming a senior developer yourself.