We need better guidance as to how to do PRs and properly review code

[Rod-at-DOH]

Select Topic Area

Question

Body

We've been using GitHub Enterprise, with an Organization where we keep all of our code, for about two years now. For the most part, it is going well, but I do see some problems related to PRs, reviewing code, creating code, and merging from one branch to another with a PR.

I've taken the GitHub skill that covered doing pull requests. That was a good, basic tutorial. I don't know if my colleagues have taken it or not. I suspect that a few have, but the majority haven't. It's been a few years since I took that skill, so I don't remember all the details, but if memory serves it showed how to create a PR in the GitHub.com interface, assigning it to a colleague. Or accepting a PR assigned to you. Some basic merge conflict resolutions skills, etc. Pretty straight forward.

But that doesn't cover what I sometimes see here. What I've seen is a colleague will create a branch in a repo. Then he'll do work in that branch and create a PR to merge changes back to main. All fine and good. But he'll leave that branch around for weeks, even months. Later, when other changes need to be merged into main. He will duplicate whatever was done in some other branch into his branch, which is now weeks or months out of date. And then he'll create a PR, someone else will review it and merge all that stuff into main.

I'm sure that is causing us problems. For example, today I've put the target="_blank" into the anchor tag on a web page. For the third time, because it has been removed by other PRs wiping it out. And I'm sure that this colleague and others who do the same thing, are what's causing the problem.

The GitHub skill to teach basic PR and code review, doesn't cover this situation at all. And colleagues do this over and over again. We spend a fair amount of rework, because of this. Is there a better, more advanced GitHub skill that will cover how to avoid re-introducing code that has been replaced, with old code deemed wrong?

[Arondith]

A simple team workflow that usually works well is:

1. Keep pull requests small and focused.
   Try to make each PR solve one issue or one clearly defined change. This makes reviews faster and easier to understand.

2. Give reviewers enough context.
   In the PR description, explain:

What problem is being solved
What changed
How the change was tested
Any risks or areas the reviewer should pay attention to

3. Review the purpose before the code.
   Before looking line by line, make sure the change actually solves the intended problem.

4. Review the Files changed tab systematically.
   Check each file for correctness, readability, edge cases, security concerns, duplicated logic, and whether tests need to be updated.

5. Use GitHub’s review options properly.

Comment for suggestions or questions
Approve when the PR is ready
Request changes when something should be fixed before merging

6. Require reviews and status checks on important branches.
   Protecting main can prevent accidental direct pushes and require approvals or CI checks before merging.

7. Resolve review conversations before merging.
   If changes are made after a review, the author can request another review so the latest version is checked.

For a team that is still learning GitHub, I would also create a short internal PR checklist and use the same workflow for every repository. Consistency helps much more than trying to create a complicated process immediately.

GitHub’s pull request review documentation and review quickstart are also good references for the team.

[edwinsatya]

What you are experiencing isn't a lack of GitHub UI training—it's a process and hygiene issue known as branch drift and improper merge conflict resolution.

No course on GitHub.com will fix this on its own, because the interface allows people to force bad code into main if team conventions and repository safeguards aren't enforced.

Here is what is happening mechanically, why it keeps wiping out your code, and the specific controls you can put in place to stop it.

---

What Your Colleague Is Doing Wrong

When a branch stays open for weeks or months, its history freezes in time while main keeps moving forward. When this developer reuses that stale branch for new work:

1. They are overwriting newer changes with older state. If they edit a file locally using an outdated version of that file as their base, Git (or the developer during a bad manual conflict resolution) assumes their version is the intended truth.
2. They are treating branches like persistent workspaces instead of disposable task units. Branches should be created for a single issue or ticket, merged, and immediately deleted.
3. They are bypassing proper branch synchronization. Instead of pulling down main and rebasing their feature branch onto it before opening a PR, they are forcing diverged changes together.

---

How to Fix This in Your GitHub Organization

To eliminate code regressions like your missing target="_blank", you need a combination of GitHub Enterprise settings and strict team workflow rules.

1. Turn On "Require Branches to Be Up to Date Before Merging"

In your repository settings (Settings > Branches > Branch protection rules for main):

• Enable Require status checks to pass before merging.
• Check Require branches to be up to date before merging.

What this does: GitHub will literally disable the "Merge" button on any PR whose base branch (main) has moved ahead since the PR branch was created. The developer will be forced to click "Update branch" (or rebase locally) and resolve conflicts against current main before anyone can merge.

2. Auto-Delete Merged Branches

In repository settings (Settings > General > Pull Requests):

• Check Automatically delete head branches.

What this does: As soon as a PR merges, GitHub deletes the remote feature branch. This prevents developers from casually pulling from an old branch and stacking new work on top of dead history.

3. Establish a Short-Lived Branch Policy

Adopt a standard workflow (like GitHub Flow or Trunk-Based Development) with three non-negotiable team rules:

• One branch per ticket/task: Never mix work from different features in one branch.
• Max lifespan: If a branch is older than a sprint (1–2 weeks), it must be rebased onto main or closed.
• Never reuse merged or closed branches: Fresh task = new branch checked out directly from the latest main.

4. Use CODEOWNERS and Strict Reviews

Add a CODEOWNERS file to your repository root. You can assign specific senior devs or team leads to automatically be requested as reviewers for critical files or directories. When reviewing PRs, reviewers must check the Files Changed tab specifically looking for deleted lines that shouldn't be touched.

5. Add Automated Tests / Linters

If a specific piece of code keeps disappearing (like HTML attributes, security headers, or configuration settings), write an automated test or linter rule in your CI pipeline. If a PR accidentally deletes required code, the CI build turns red and blocks the merge automatically.

---

Recommended Learning Path

If you want your team to complete structured training through GitHub Skills, point them to these specific modules rather than basic PR tutorials:

1. GitHub Skills: Resolving Merge Conflicts – Teaches how Git tracks line changes and how to properly inspect and resolve overlapping code.
2. GitHub Skills: Managing Protected Branches – Good for repository admins to understand enforcement settings.
3. Pro Git Book (Chapter 3: Git Branching) – Free online resource. Sections 3.2 (Basic Branching and Merging) and 3.6 (Rebasing) explain why stale branches cause history corruption and how git rebase solves it.