Skip to main content
Redhat Developers  Logo
  • Products

    Featured

    • Red Hat Enterprise Linux
      Red Hat Enterprise Linux Icon
    • Red Hat OpenShift AI
      Red Hat OpenShift AI
    • Red Hat Enterprise Linux AI
      Linux icon inside of a brain
    • Image mode for Red Hat Enterprise Linux
      RHEL image mode
    • Red Hat OpenShift
      Openshift icon
    • Red Hat Ansible Automation Platform
      Ansible icon
    • Red Hat Developer Hub
      Developer Hub
    • View All Red Hat Products
    • Linux

      • Red Hat Enterprise Linux
      • Image mode for Red Hat Enterprise Linux
      • Red Hat Universal Base Images (UBI)
    • Java runtimes & frameworks

      • JBoss Enterprise Application Platform
      • Red Hat build of OpenJDK
    • Kubernetes

      • Red Hat OpenShift
      • Microsoft Azure Red Hat OpenShift
      • Red Hat OpenShift Virtualization
      • Red Hat OpenShift Lightspeed
    • Integration & App Connectivity

      • Red Hat Build of Apache Camel
      • Red Hat Service Interconnect
      • Red Hat Connectivity Link
    • AI/ML

      • Red Hat OpenShift AI
      • Red Hat Enterprise Linux AI
    • Automation

      • Red Hat Ansible Automation Platform
      • Red Hat Ansible Lightspeed
    • Developer tools

      • Red Hat Trusted Software Supply Chain
      • Podman Desktop
      • Red Hat OpenShift Dev Spaces
    • Developer Sandbox

      Developer Sandbox
      Try Red Hat products and technologies without setup or configuration fees for 30 days with this shared Openshift and Kubernetes cluster.
    • Try at no cost
  • Technologies

    Featured

    • AI/ML
      AI/ML Icon
    • Linux
      Linux Icon
    • Kubernetes
      Cloud icon
    • Automation
      Automation Icon showing arrows moving in a circle around a gear
    • View All Technologies
    • Programming Languages & Frameworks

      • Java
      • Python
      • JavaScript
    • System Design & Architecture

      • Red Hat architecture and design patterns
      • Microservices
      • Event-Driven Architecture
      • Databases
    • Developer Productivity

      • Developer productivity
      • Developer Tools
      • GitOps
    • Secure Development & Architectures

      • Security
      • Secure coding
    • Platform Engineering

      • DevOps
      • DevSecOps
      • Ansible automation for applications and services
    • Automated Data Processing

      • AI/ML
      • Data Science
      • Apache Kafka on Kubernetes
      • View All Technologies
    • Start exploring in the Developer Sandbox for free

      sandbox graphic
      Try Red Hat's products and technologies without setup or configuration.
    • Try at no cost
  • Learn

    Featured

    • Kubernetes & Cloud Native
      Openshift icon
    • Linux
      Rhel icon
    • Automation
      Ansible cloud icon
    • Java
      Java icon
    • AI/ML
      AI/ML Icon
    • View All Learning Resources

    E-Books

    • GitOps Cookbook
    • Podman in Action
    • Kubernetes Operators
    • The Path to GitOps
    • View All E-books

    Cheat Sheets

    • Linux Commands
    • Bash Commands
    • Git
    • systemd Commands
    • View All Cheat Sheets

    Documentation

    • API Catalog
    • Product Documentation
    • Legacy Documentation
    • Red Hat Learning

      Learning image
      Boost your technical skills to expert-level with the help of interactive lessons offered by various Red Hat Learning programs.
    • Explore Red Hat Learning
  • Developer Sandbox

    Developer Sandbox

    • Access Red Hat’s products and technologies without setup or configuration, and start developing quicker than ever before with our new, no-cost sandbox environments.
    • Explore Developer Sandbox

    Featured Developer Sandbox activities

    • Get started with your Developer Sandbox
    • OpenShift virtualization and application modernization using the Developer Sandbox
    • Explore all Developer Sandbox activities

    Ready to start developing apps?

    • Try at no cost
  • Blog
  • Events
  • Videos

10 tips for reviewing code you don't like

July 8, 2019
David Lloyd
Related topics:
Open source

Share:

    As a frequent contributor to open source projects (both within and beyond Red Hat), I find one of the most common time-wasters is dealing with code reviews of my submitted code that are negative or obstructive and yet essentially subjective or argumentative in nature. I see this most often when submitting to projects where the maintainer doesn't like the change, for whatever reason. In the best case, this kind of code review strategy can lead to time wasted in pointless debates; at worst, it actively discourages contribution and diversity in a project and creates an environment that is hostile and elitist.

    A code review should be objective and concise and should deal in certainties whenever possible. It's not a political or emotional argument; it's a technical one, and the goal should always be to move forward and elevate the project and its participants.  A change submission should always be evaluated on the merits of the submission, not on one's opinion of the submitter.

    Code review strategies

    Here are several strategies to keep in mind when reviewing submissions that, for whatever reason, you (as a project maintainer) do not like:

    1. Rephrase your objection as a question

    • Bad: "This change will make XXX impossible." (This is hyperbole; is it really impossible?)
    • Good: "How can we do XXX with your change?"

    2. Avoid hyperbole

    Simply state your concerns and ask questions to help get to the desired outcome.

    • Bad: "This change will destroy performance."
    • Good: "It seems like doing X might be slower than existing Y; have you measured/gathered data to show it isn't?"
    • Better (if you have time): "In the meantime, I am gathering data to try to verify that X is not slower than Y."
    • Also good: "This change changes this single loop O(n) to a doubly nested loop O(n²); won't this affect performance?"

    3. Keep snide comments to yourself

    Some thoughts are better kept to yourself. If you can't be civil, don't engage.

    • Bad: "I think this change is bad and will ruin everything."
    • Bad: "Are you sure that software engineering is the right career path for you?"

    4. Engage positively

    Maybe you had a different idea about how to solve a problem? If you engage positively, you might end up discovering a solution that is better than either original option.

    • Bad: "This change sucks, my version is better."
    • Good: "I also have a similar change at this location XXX: maybe we can compare and/or combine ideas."
    • Also good: "I have a similar change in progress, but I chose to do X because ZZZ; why did you choose Y?"

    5. Remember that not everybody's experience is identical to yours

    An otherwise completely competent engineer could go for years without knowing some fact that you take as common sense. It's okay to state the obvious, as long as you aren't patronizing or snide about it.

    • Bad: "Can't you see that this is obviously wrong?"
    • Good: "This is incorrect because it causes a null pointer exception when X is Y."

    6. Don't diminish the complexity of something that's not obvious

    Remember that things that are obvious to you may not be obvious to everyone. Suggesting alternative approaches and pointing out useful examples can help get everyone on the same page.

    • Bad: "Why not simply frob the gnozzle?"
    • Good: "It might be possible to frob the gnozzle, which would simplify this part (see XXX for an example)."

    7. Be respectful

    Sometimes a submission just doesn't meet a minimum standard for quality. It's okay to say so, but it doesn't cost anything extra to be respectful.

    • Bad: "This is stupid code written by a stupid person."
    • Good: "Thanks for your contribution. However, it cannot be accepted in its current form; there are multiple problems (as outlined above)."
    • Also good: "As outlined above, there are multiple problems with this submission.  Maybe we could back up a step and talk about the use cases instead?  That could help us find a path forward."

    8. Manage expectations (and your time)

    If a submission is too large to be reasonably reviewed, it is okay to let the submitter know right away. Keep moving forward.

    • Bad: "I'm not merging this, it's too big."
    • Also bad: Ignoring it until it goes away.
    • Good: "Could you please break this down into smaller changes? I do not have a lot of time for code reviews and this one is just too large/complex to review in one pass."

    9. Say please

    Just saying "please" goes a long way toward showing that you respect the submitter's time, especially when you want something to be different due to formatting or style, which might seem to be a minor detail of the change. Examples:

    • "Could you please separate the whitespace changes into another pull request?"
    • "Could you please align these variable definitions so they're easier to read?"

    10. Start a conversation

    If, after all this, you still don't like something but you're not sure why, you might have to just live with it. But it's also okay to say, "I don't like this and I'm not sure why, can we talk about it?" It's a reasonable thing to ask, and even though it might take a little time, it's often worth the investment because now you have two people who are both learning (one by explaining and one by listening) rather than two people who are opposed to each other.

    Even skilled and experienced engineers should be able to say "I don't understand why I don't like this"; it's not an invitation to attack the position of the reviewer but rather an honest quest for knowledge.

    Summary

    Avoid hyperbolic or bombastic assertions, avoid argument strategies, avoid elitist or demeaning language, and avoid constructs like "obviously" and "why don't you just...".  Use clear, factual statements and supportive language, ask questions, and move things forward.  Remember that coworkers and contributors are human people, and their time is worthy of the same respect as yours.

    Last updated: July 15, 2019

    Recent Posts

    • How Kafka improves agentic AI

    • How to use service mesh to improve AI model security

    • How to run AI models in cloud development environments

    • How Trilio secures OpenShift virtual machines and containers

    • How to implement observability with Node.js and Llama Stack

    Red Hat Developers logo LinkedIn YouTube Twitter Facebook

    Products

    • Red Hat Enterprise Linux
    • Red Hat OpenShift
    • Red Hat Ansible Automation Platform

    Build

    • Developer Sandbox
    • Developer Tools
    • Interactive Tutorials
    • API Catalog

    Quicklinks

    • Learning Resources
    • E-books
    • Cheat Sheets
    • Blog
    • Events
    • Newsletter

    Communicate

    • About us
    • Contact sales
    • Find a partner
    • Report a website issue
    • Site Status Dashboard
    • Report a security problem

    RED HAT DEVELOPER

    Build here. Go anywhere.

    We serve the builders. The problem solvers who create careers with code.

    Join us if you’re a developer, software engineer, web designer, front-end designer, UX designer, computer scientist, architect, tester, product manager, project manager or team lead.

    Sign me up

    Red Hat legal and privacy links

    • About Red Hat
    • Jobs
    • Events
    • Locations
    • Contact Red Hat
    • Red Hat Blog
    • Inclusion at Red Hat
    • Cool Stuff Store
    • Red Hat Summit

    Red Hat legal and privacy links

    • Privacy statement
    • Terms of use
    • All policies and guidelines
    • Digital accessibility

    Report a website issue