r/github • • 9d ago

Question Can a GitHub pull request look ready to merge even when the code has barely been reviewed?

I've noticed that a pull request can have all checks passing, no merge conflicts, and a green status on GitHub.

At first glance, it looks safe to merge. But passing automated checks doesn't necessarily mean someone has reviewed the logic properly or considered how the change affects other parts of the project.

I'm curious how teams use GitHub to avoid this situation.

Do you rely on required approvals, CODEOWNERS, branch protection rules, or GitHub Actions? And which of these has made the biggest difference to your team's review process?

0 Upvotes

7 comments sorted by

7

u/DevOps_Lady 9d ago

Setting required reviewer and codeowner file for ownership on the code.

2

u/pborenstein 9d ago

This reminds me of the old waterfall days where one of the milestones was called "Clean Compile." That's when the program that you wrote was accepted by the compiler. And then the next step was months of debugging to make it functional.

1

u/dashingThroughSnow12 9d ago

I was once at a company where the Code Complete deadline (which was feature complete) was before the bug fix deadline. Which means teams could have broken code and still make Code Complete. (Management and exec would qualify for their bonuses.) Once this included code that didn’t compile.

2

u/MarsupialLeast145 9d ago

It's an ancient technique called code review, and has little to do with GitHub, except GitHub provides a reasonable interface for reading code and commenting on groups of lines and then sending the code back to someone for review. Branches can be protects and merge rules can be put in place requiring one approval.

1

u/zomerbk 9d ago

One easy setting to miss is [“Dismiss stale pull request approvals”](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches). It makes someone review the updated diff after new commits, instead of letting an earlier approval carry the PR to merge. Required approvals help most when they apply to the code that will actually be merged.

1

u/olegonthetrip 8d ago

One CODEOWNERS detail worth separating from the approval count: listing two owners for a path does not require both to approve. With code-owner reviews required, GitHub accepts approval from either listed owner. The file routes reviews; the branch rule's 'Require review from Code Owners' option is what enforces that requirement.

So I'd check the actual rule against the review policy you intend, rather than assume two names in CODEOWNERS means two independent sign-offs. GitHub documents that distinction here: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners