Skip to content

docs: prefer upstream PR branches in code review guidance#382

Merged
enyst merged 5 commits intomainfrom
openhands/code-review-upstream-pr-branch
Mar 6, 2026
Merged

docs: prefer upstream PR branches in code review guidance#382
enyst merged 5 commits intomainfrom
openhands/code-review-upstream-pr-branch

Conversation

@enyst
Copy link
Collaborator

@enyst enyst commented Mar 6, 2026

Summary

  • add guidance to validate docs against referenced upstream PR branches
  • update verification example to include cloning PR head branches

Testing

  • Not run (docs-only guidance)

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst marked this pull request as ready for review March 6, 2026 05:01
Copy link
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taste Rating: 🟡 Acceptable - Solves a real problem (false positive reviews when docs match upstream PRs) with a simple solution. Minor clarity issues that could confuse readers.

Verdict: ✅ Worth merging after addressing the inconsistency

Key Insight: The grep example contradicts the new guidance by still searching the main branch instead of the PR branch.

Co-authored-by: openhands <openhands@all-hands.dev>
Copy link
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taste Rating: 🟡 Acceptable - Solves a real problem (false positive reviews when docs correctly match upstream PRs) with a straightforward solution. One minor clarity improvement suggested.

Verdict: ✅ Worth merging after addressing the ambiguity, or merge as-is if you think the instruction is clear enough.

Key Insight: The "replace" instruction could be misread as "edit this file" rather than "run this command instead of that one."

Co-authored-by: OpenHands Bot <contact@all-hands.dev>
Copy link
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taste Rating: 🟡 Acceptable - Solves a real workflow problem (false positive reviews when docs correctly match upstream PRs) with a straightforward solution.

Verdict: ✅ Ready to merge

Key Insight: Pragmatic documentation fix that aligns reviewer behavior with the actual sync workflow - check the right source of truth when docs intentionally lead upstream changes.

@enyst enyst merged commit a2022f5 into main Mar 6, 2026
6 checks passed
@enyst enyst deleted the openhands/code-review-upstream-pr-branch branch March 6, 2026 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants