r/github • u/No_Ambition8323 • 1d 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?
2
u/pborenstein 1d 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 1d 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 1d 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 1d 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/AirshipCommand 1d ago
Besides required approvals and CODEOWNERS, turn on "Require approval of the most recent reviewable push" in the branch rules, so an approval doesn't carry over to commits pushed after it. A PR template with a short "what could this break" section also helps, since it gives the reviewer something specific to check beyond the diff.
1
u/olegonthetrip 20h 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
7
u/DevOps_Lady 1d ago
Setting required reviewer and codeowner file for ownership on the code.