Think Twice Before Merging: Green Doesn't Always Mean Go

As the co-lead for the Android team, I have dedicated a significant amount of time to refining my approach to code reviews. What initially seemed like a simple task—checking logic, ensuring best practices, and confirming that the code functions as expected—quickly became more complex. While it started as a checklist to tick off, I soon realized that code reviews are much more than just verifying the basics. They require a deeper understanding of how new code interacts with the rest of the system, especially when reviewing code that wasn’t originally written by me.

One experience stands out vividly. I was reviewing a colleague’s code, and at first glance, everything seemed to be in order. But as I dug deeper, I identified several issues that needed attention. I reached out to discuss these concerns, expecting a constructive conversation. Instead, I was told, "It’s green on GitHub, so it should be fine. Just merge it."

While that response seemed logical—after all, the tests had passed and the build was green—I didn’t settle for just that. I took the time to manually test the changes and uncovered an issue: the code failed when merged with the main branch, something the automated tests hadn’t picked up. If I had merged it as suggested, it would have disrupted the main branch and potentially broken existing functionality.

I insisted that they review their code again, and, sure enough, they discovered the issue. That experience was an eye-opener. It taught me a vital lesson: just because a branch passes automated tests and shows a green checkmark does not mean it's safe to merge.

This experience reinforced the critical value of thorough code reviews. Here’s why:

The Importance of Thorough Code Reviews:

Here’s the list in bullet points:

• Automated Tests Aren't Perfect: While CI/CD pipelines and automated tests catch a lot, they don't cover every edge case. Manual testing still plays a crucial role.

• Your Job is to Protect Main: The main branch should always remain stable. A rushed or careless merge can introduce critical bugs.

• Understanding the Code Matters: Even if the logic looks fine at first glance, take the time to deeply understand what the code does, how it interacts with existing functionality, and whether it aligns with project standards.

• Speak Up When Something Feels Off: As a reviewer, you are the last line of defense. If something doesn’t seem right, ask questions and validate before approving. Trust your gut if something feels off because it will be on you if you merge it.

To conclude, code reviews are more than just a routine task—they are a responsibility. A single unchecked issue can lead to significant problems down the road. It’s essential to take the time to verify, test, and engage in discussions with the author to ensure the code is truly ready. Not everything that’s marked as "green" means it's good to go. Blindly merging without careful review can jeopardize the entire project.

The next time you review a pull request, remember: your job isn’t just to approve the code but to make sure it's truly ready. 🚀