--- title: "Reviewing a Pull Request" url: https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr/ --- # Reviewing a Pull Request This document uses key words such as "MUST", "SHOULD", and "MAY" as defined in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt) to indicate requirement levels. Statuscandidate Version`1.0.0`[(changelog)](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#changelog) ## [Overview](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#overview) This playbook guides reviewers through conducting effective code reviews for SDK pull requests. It covers PR description validation, CI verification, code quality assessment, and feedback conventions using the LOGAF scale. By following these steps, reviews will focus on risk reduction, maintain consistent quality standards, and provide actionable feedback. Related resources: * [Review and CI Standards](https://develop.sentry.dev/sdk/getting-started/standards/review-ci.md) — review requirements and feedback conventions * [Code Quality Standards](https://develop.sentry.dev/sdk/getting-started/standards/code-quality.md) — test requirements and quality criteria * [Sentry code review checklist](https://develop.sentry.dev/engineering-practices/code-review.md) — detailed review criteria * [Sentry Skills](https://github.com/getsentry/skills#available-skills) — code-review skill for automated checks *** ## [Steps](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#steps) #### [1. Check the PR description](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#1-check-the-pr-description) Does it explain what and why? Is there a linked issue? If either is missing, request it before reviewing code. #### [2. Check CI status and prior AI/human reviews](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#2-check-ci-status-and-prior-aihuman-reviews) A reviewer **SHOULD NOT** begin a full review unless all of the following conditions are met: * All CI checks, builds, and tests have completed successfully. * All requested AI code review agents have completed their reviews. * All comments from previous review cycles (AI or human) have been addressed, resolved, or explicitly marked as not applicable. If any of the above conditions are unmet, the reviewer **MAY** leave comments to help the author unblock CI failures or address outstanding feedback. #### [3. Review the code](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#3-review-the-code) Use the [code review checklist](https://develop.sentry.dev/engineering-practices/code-review.md). You **MUST** focus on: * Runtime errors and potential crashes * Performance concerns (N+1 queries, unbounded operations) * Unintended side effects or behavior changes * Backwards compatibility * Security vulnerabilities * Test coverage appropriate for the change type ([Test requirements by change type](https://develop.sentry.dev/sdk/getting-started/standards/code-quality.md#testing)) * Test quality — do assertions verify real behavior? ([Test quality](https://develop.sentry.dev/sdk/getting-started/standards/code-quality.md#testing)) You **SHOULD** use the [`sentry-skills:code-review`](https://github.com/getsentry/skills#available-skills) skill to systematically check for issues. #### [4. Check for @sdk-seniors review triggers](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#4-check-for-sdk-seniors-review-triggers) ([Required reviewers](https://develop.sentry.dev/sdk/getting-started/standards/review-ci.md#code-review)): public API changes, new dependencies, schema changes, security-sensitive code, new frameworks. If any apply and no @sdk-seniors reviewer is assigned, flag it. #### [5. Use LOGAF prefixes on all feedback](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#5-use-logaf-prefixes-on-all-feedback) You **MUST** use LOGAF prefixes on all feedback ([Review feedback conventions](https://develop.sentry.dev/sdk/getting-started/standards/review-ci.md#code-review)): * `h:` (high) — must fix before merge. Bugs, security issues, breakage, data loss. * `m:` (medium) — should fix. Design concerns, missing tests, unclear code. * `l:` (low) — optional nit. Style preferences, minor suggestions. #### [6. Approve when only `l:` items remain](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#6-approve-when-only-l-items-remain) You **MUST NOT** block for style preferences. The goal is risk reduction, not perfection. ## [Referenced Standards](https://develop.sentry.dev/sdk/getting-started/playbooks/development/reviewing-a-pr.md#referenced-standards) * [Review feedback conventions](https://develop.sentry.dev/sdk/getting-started/standards/review-ci.md#code-review) — LOGAF scale and blocking criteria * [Required reviewers](https://develop.sentry.dev/sdk/getting-started/standards/review-ci.md#code-review) — @sdk-seniors review triggers * [Required CI checks baseline](https://develop.sentry.dev/sdk/getting-started/standards/review-ci.md#required-ci-checks) — minimum CI requirements * [Test requirements by change type](https://develop.sentry.dev/sdk/getting-started/standards/code-quality.md#testing) — test coverage expectations * [Test quality](https://develop.sentry.dev/sdk/getting-started/standards/code-quality.md#testing) — meaningful assertion requirements * [PR description quality](https://develop.sentry.dev/sdk/getting-started/standards/code-submission.md#pr-description-quality) — description content requirements *** | Version | Date | Summary | | ------- | ---------- | ----------------------------------------------------------------------------------- | | `1.0.0` | 2026-02-21 | Initial playbook — standardized code review process with LOGAF feedback conventions |