Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ This playbook guides SDK maintainers through the process of adding a new depende

Related resources:
- [Dependency Management Standard](/sdk/getting-started/standards/code-quality#dependency-management) — dependency requirements and audit practices
- [SDK Size and Performance Budgets](/sdk/getting-started/standards/code-quality#sdk-size-and-performance-budgets) — size and performance tracking requirements
- [SDK Size and Performance Budgets](/sdk/getting-started/standards/code-quality#size-performance-budgets) — size and performance tracking requirements
- [Security Practices](/sdk/getting-started/standards/code-quality#security-practices) — security requirements for dependencies

---
Expand All @@ -40,7 +40,7 @@ The issue **MUST** include documentation of the following:
- Maintenance health (last release date, number of maintainers, open issue count)
- Security posture (known vulnerabilities, audit history)
- License compatibility
- Size impact on the SDK (see [SDK size and performance budgets](/sdk/getting-started/standards/code-quality#sdk-size-and-performance-budgets))
- Size impact on the SDK (see [SDK size and performance budgets](/sdk/getting-started/standards/code-quality#size-performance-budgets))

#### 3. Get explicit approval

Expand All @@ -65,7 +65,7 @@ The PR reviewer **MUST** explicitly acknowledge the new dependency, not just app
## Referenced Standards

- [Dependency Management](/sdk/getting-started/standards/code-quality#dependency-management) — requirements for adding and maintaining dependencies
- [SDK Size and Performance Budgets](/sdk/getting-started/standards/code-quality#sdk-size-and-performance-budgets) — size and performance budget tracking
- [SDK Size and Performance Budgets](/sdk/getting-started/standards/code-quality#size-performance-budgets) — size and performance budget tracking
- [Security Practices](/sdk/getting-started/standards/code-quality#security-practices) — security requirements and vulnerability handling

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ If the PR touches public API, dependencies, or security-sensitive areas, you **M
- [One logical change per PR](/sdk/getting-started/standards/code-submission#one-logical-change-per-pr) — focused PR scope
- [Documentation-with-code](/sdk/getting-started/standards/repository-docs#documentation-with-code) — docs PR requirements
- [Required reviewers](/sdk/getting-started/standards/review-ci#required-reviewers)@sdk-seniors review triggers
- [Test requirements by change type](/sdk/getting-started/standards/code-quality#test-requirements-by-change-type) — test coverage requirements
- [Test requirements by change type](/sdk/getting-started/standards/code-quality#testing) — test coverage requirements

---

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ Use the [code review checklist](/engineering-practices/code-review/). You **MUST
- Unintended side effects or behavior changes
- Backwards compatibility
- Security vulnerabilities
- Test coverage appropriate for the change type ([Test requirements by change type](/sdk/getting-started/standards/code-quality#test-requirements-by-change-type))
- Test quality — do assertions verify real behavior? ([Test quality](/sdk/getting-started/standards/code-quality#test-quality))
- Test coverage appropriate for the change type ([Test requirements by change type](/sdk/getting-started/standards/code-quality#testing))
- Test quality — do assertions verify real behavior? ([Test quality](/sdk/getting-started/standards/code-quality#testing))

You **SHOULD** use the [`sentry-skills:code-review`](https://github.com/getsentry/skills#available-skills) skill to systematically check for issues.

Expand All @@ -81,8 +81,8 @@ You **MUST NOT** block for style preferences. The goal is risk reduction, not pe
- [Review feedback conventions](/sdk/getting-started/standards/review-ci#review-feedback-conventions) — LOGAF scale and blocking criteria
- [Required reviewers](/sdk/getting-started/standards/review-ci#required-reviewers) — @sdk-seniors review triggers
- [Required CI checks baseline](/sdk/getting-started/standards/review-ci#required-ci-checks-baseline) — minimum CI requirements
- [Test requirements by change type](/sdk/getting-started/standards/code-quality#test-requirements-by-change-type) — test coverage expectations
- [Test quality](/sdk/getting-started/standards/code-quality#test-quality) — meaningful assertion requirements
- [Test requirements by change type](/sdk/getting-started/standards/code-quality#testing) — test coverage expectations
- [Test quality](/sdk/getting-started/standards/code-quality#testing) — meaningful assertion requirements
- [PR description quality](/sdk/getting-started/standards/code-submission#pr-description-quality) — description content requirements

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ You **MUST NOT** review failing code.

#### 3. Review for common issues

Runtime errors, performance, side effects, backwards compatibility, security, test coverage ([Test requirements by change type](/sdk/getting-started/standards/code-quality#test-requirements-by-change-type)), test quality ([Test quality](/sdk/getting-started/standards/code-quality#test-quality)).
Runtime errors, performance, side effects, backwards compatibility, security, test coverage ([Test requirements by change type](/sdk/getting-started/standards/code-quality#testing)), test quality ([Test quality](/sdk/getting-started/standards/code-quality#testing)).

#### 4. Check @sdk-seniors review triggers

Expand All @@ -69,7 +69,7 @@ Verify every import and function call actually exists. AI tools sometimes refere

#### 2. Tests that test nothing

You **MUST** check that test assertions would actually fail if the feature broke ([Test quality](/sdk/getting-started/standards/code-quality#test-quality)). Watch for: hardcoded expected values that happen to match the output, `assert True` or equivalents, testing mock behavior instead of real behavior, asserting only that no exception was thrown.
You **MUST** check that test assertions would actually fail if the feature broke ([Test quality](/sdk/getting-started/standards/code-quality#testing)). Watch for: hardcoded expected values that happen to match the output, `assert True` or equivalents, testing mock behavior instead of real behavior, asserting only that no exception was thrown.

#### 3. Over-engineering

Expand All @@ -92,8 +92,8 @@ You **SHOULD** use the [`sentry-skills:find-bugs`](https://github.com/getsentry/
## Referenced Standards

- [Review feedback conventions](/sdk/getting-started/standards/review-ci#review-feedback-conventions) — LOGAF scale and blocking criteria
- [Test requirements by change type](/sdk/getting-started/standards/code-quality#test-requirements-by-change-type) — test coverage expectations
- [Test quality](/sdk/getting-started/standards/code-quality#test-quality) — meaningful assertion requirements
- [Test requirements by change type](/sdk/getting-started/standards/code-quality#testing) — test coverage expectations
- [Test quality](/sdk/getting-started/standards/code-quality#testing) — meaningful assertion requirements
- [AI attribution](/sdk/getting-started/standards/code-submission#ai-attribution) — Co-Authored-By footer requirement
- [One logical change per PR](/sdk/getting-started/standards/code-submission#one-logical-change-per-pr) — focused PR scope

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ Set up CI with at minimum:

You **MUST** configure:
- Build verification
- Linting ([Linting and formatting in CI](/sdk/getting-started/standards/code-quality#linting-formatting-ci))
- Type checking, if applicable for the language ([Type checking in CI](/sdk/getting-started/standards/code-quality#type-checking-ci))
- Linting ([Linting and formatting in CI](/sdk/getting-started/standards/code-quality#linting-and-type-checking))
- Type checking, if applicable for the language ([Type checking in CI](/sdk/getting-started/standards/code-quality#linting-and-type-checking))
- Test suite
- Commit message format check ([Commit message format](/sdk/getting-started/standards/code-submission#commit-message-format))
- Secret scanning ([Security practices](/sdk/getting-started/standards/code-quality#security-practices))
Expand Down Expand Up @@ -112,8 +112,8 @@ If the tool cannot do this, the documentation needs work.
- [Required reviewers](/sdk/getting-started/standards/review-ci#required-reviewers) — review approval requirements
- [Required CI checks baseline](/sdk/getting-started/standards/review-ci#required-ci-checks) — minimum CI pipeline requirements
- [Commit message format](/sdk/getting-started/standards/code-submission#commit-message-format) — conventional commit format
- [Linting and formatting in CI](/sdk/getting-started/standards/code-quality#linting-formatting-ci) — automated code style checks
- [Type checking in CI](/sdk/getting-started/standards/code-quality#type-checking-ci) — static type verification
- [Linting and formatting in CI](/sdk/getting-started/standards/code-quality#linting-and-type-checking) — automated code style checks
- [Type checking in CI](/sdk/getting-started/standards/code-quality#linting-and-type-checking) — static type verification
- [Security practices](/sdk/getting-started/standards/code-quality#security-practices) — secret scanning and security checks
- [Rollback procedures](/sdk/getting-started/standards/coordination-maintenance#releases) — release rollback documentation

Expand Down
188 changes: 38 additions & 150 deletions develop-docs/sdk/getting-started/standards/code-quality.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
title: Code Quality
spec_id: sdk/getting-started/standards/code-quality
spec_version: 1.0.0
spec_status: candidate
spec_status: stable
spec_platforms:
- all
spec_changelog:
Expand All @@ -16,199 +16,87 @@ sidebar_order: 3

<SpecMeta />

<SpecSection id="linting-formatting-ci" status="candidate" since="1.0.0">
These standards define the baseline quality expectations for SDK code. They apply across all SDKs — specific tools and thresholds vary by language, but the principles are universal.

## Linting and Formatting in CI
<SpecSection id="linting-and-type-checking" status="stable" since="1.0.0">

#### Rule
## Linting, formatting, and type checking

Every SDK repo **MUST** have automated linting and formatting in CI. Code that doesn't pass **MUST NOT** merge. There are no exceptions to this requirement.
Every SDK repo **MUST** enforce linting and formatting in CI. Code that doesn't pass can't mergeno exceptions.

#### Enforcement
Type checking is also **REQUIRED** in CI for statically typed languages (Java, Kotlin, Swift, C#, Go, Rust, Dart) and dynamically typed languages with established type-checking ecosystems (TypeScript, Python, PHP, Elixir). Languages without mature type-checking tooling (e.g., Ruby) may be exempt.

- CI required status check (language-specific linters)

#### Suggested skill(s)

- [`sentry-skills:claude-settings-audit`](https://github.com/getsentry/skills#available-skills) - detects tech stack and recommends linter configuration.

#### Per-SDK override

<StatusBadge type="yes" /> Linter choice, rules, strictness. The standard is
that a linter exists and is enforced.

</SpecSection>

---

<SpecSection id="type-checking-ci" status="candidate" since="1.0.0">

## Type Checking in CI

#### Rule

Type checking is **REQUIRED** in CI for:

- All statically typed languages (Java, Kotlin, Swift, C#, Go, Rust, Dart)
- Dynamically typed languages with an established type-checking ecosystem (TypeScript, Python, PHP, Elixir)

Dynamically typed languages without an established type-checking system (e.g., Ruby) are TBD or **MAY** be exempt with a valid reason.

#### Enforcement

- CI required status check

#### Per-SDK override

<StatusBadge type="yes" /> Configuration and strictness. Gradual typing allowed
with a tracked plan.

</SpecSection>

---

<SpecSection id="test-requirements" status="candidate" since="1.0.0">

## Test Requirements by Change Type

#### Rule

- **Bug fix**: regression test (fails without fix, passes with it).
- **New feature**: unit tests + integration tests where applicable.
- **Refactor**: existing tests pass; no new tests required unless coverage was missing.
- **Performance change**: benchmark or measurable evidence.
- **Breaking change**: update/remove old tests, add new tests.

#### Enforcement

- AGENTS.md instruction
- Human review

#### Suggested skill(s)

- [`sentry-skills:code-review`](https://github.com/getsentry/skills#available-skills) - includes test coverage in checklist
- [`sentry-skills:find-bugs`](https://github.com/getsentry/skills#available-skills) - identifies untested paths

#### Per-SDK override

<StatusBadge type="yes" /> Test categories and boundaries are SDK-specific.
Which linter, what rules, and how strict — that's up to each SDK. The standard is that automated checks exist and block merges.

</SpecSection>

---

<SpecSection id="test-quality" status="candidate" since="1.0.0">
<SpecSection id="testing" status="stable" since="1.0.0">

## Test Quality
## Testing

#### Rule
### What to test

Tests **MUST** assert meaningful behavior:
The type of change determines what tests are needed:

- Assertions **MUST** fail if the tested behavior breaks
- "No exception thrown" assertions are only acceptable when exception handling is the behavior being tested
- Snapshot tests **MUST** be reviewed for correctness, not auto-accepted
- AI-generated tests **REQUIRE** extra scrutiny for:
- Hardcoded values
- Implementation-detail testing
- Tests that pass regardless of behavior
- **Bug fix**: a regression test that fails without the fix and passes with it
- **New feature**: unit tests, plus integration tests where applicable
- **Refactor**: existing tests should still pass; add new tests only if coverage was missing
- **Performance change**: benchmark or measurable evidence
- **Breaking change**: update or remove old tests, add new ones

#### Enforcement
### Test quality

- AGENTS.md: "Every test must answer: what user-visible behavior would break if this test were deleted?"
- Human review
Tests should assert meaningful behavior. Ask yourself: *what user-visible behavior would break if this test were deleted?* If the answer is "nothing," the test isn't useful.

#### Suggested skill(s)
Some specific things to watch for:

- [`sentry-skills:code-review`](https://github.com/getsentry/skills#available-skills)
- [`sentry-skills:find-bugs`](https://github.com/getsentry/skills#available-skills)

#### Per-SDK override

<StatusBadge type="no" /> Principle is universal. Language-specific
anti-patterns documented per-SDK in AGENTS.md.
- "No exception thrown" is only a valid assertion when exception handling is the behavior under test
- Snapshot tests need to be reviewed for correctness, not auto-accepted
- AI-generated tests need extra scrutiny — look for hardcoded values, implementation-detail testing, and tests that pass regardless of behavior

</SpecSection>

---

<SpecSection id="dependency-management" status="candidate" since="1.0.0">

## Dependency Management

#### Rule

- New dependency **REQUIRES** explicit justification in the PR description
- Dependencies **MUST** be pinned to a version range (not floating versions)
- Transitive dependency behavior changes **MUST** receive the same scrutiny as direct dependency additions
- Dependency audits **MUST** be conducted quarterly at minimum

#### Enforcement
<SpecSection id="dependency-management" status="stable" since="1.0.0">

- CI detection of new dependency manifest entries
- Human review
## Dependencies

#### Suggested skill(s)
Adding a dependency is a decision that affects every user of the SDK, so it deserves scrutiny. New dependencies **REQUIRE** explicit justification in the PR description. Pin them to a version range — no floating versions.

- [`sentry-skills:security-review`](https://github.com/getsentry/skills#available-skills)
Transitive dependencies matter too. If a transitive dependency changes behavior, that deserves the same review as adding a direct dependency. Run dependency audits at least quarterly.

See also: [Adding a Dependency](/sdk/getting-started/playbooks/development/adding-a-dependency) for the step-by-step approval and evaluation workflow.

#### Per-SDK override

<StatusBadge type="yes" /> Tooling varies. Approval requirement is universal.
See [Adding a Dependency](/sdk/getting-started/playbooks/development/adding-a-dependency) for the step-by-step evaluation and approval workflow.

</SpecSection>

---

<SpecSection id="size-performance-budgets" status="candidate" since="1.0.0">

## SDK Size and Performance Budgets

#### Rule

Each SDK **MUST** define and track:

- Package/binary size budget
- Initialization time budget
- Memory overhead budget (where applicable)
<SpecSection id="size-performance-budgets" status="stable" since="1.0.0">

#### Enforcement
## Size and performance budgets

- CI check where feasible (size comparison, benchmarks); advisory initially, path toward blocking
Each SDK **MUST** define and track budgets for:

#### Per-SDK override
- Package/binary size (where applicable)
- Initialization time
- Memory overhead

<StatusBadge type="yes" /> Budgets and measurement are SDK-specific. The
standard is that budgets exist and are tracked.
The specific numbers are up to each SDK. The standard is that budgets exist, are tracked, and are visible in CI.

</SpecSection>

---

<SpecSection id="security-practices" status="candidate" since="1.0.0">

## Security Practices

#### Rule

No secrets, credentials, or PII in code or commits. Data handling follows the [data handling spec](/sdk/foundations/data-scrubbing/). Security-sensitive changes **REQUIRE** explicit review from someone with security context. Vulnerability patch SLAs: critical 48h, high 1 week, medium next release. Dependency vulnerability alerts **MUST NOT** be ignored or silently dismissed.

#### Enforcement

- CI secret scanning
- Dependency scanning (GitHub Security Advisories / Dependabot alerts)
- Human review

#### Suggested skill(s)
<SpecSection id="security-practices" status="stable" since="1.0.0">

- [`sentry-skills:security-review`](https://github.com/getsentry/skills#available-skills)
- [`sentry-skills:find-bugs`](https://github.com/getsentry/skills#available-skills)
## Security

#### Per-SDK override
No secrets, credentials, or PII in code or commits. Data handling follows the [data handling spec](/sdk/foundations/data-scrubbing/).

<StatusBadge type="yes" /> Vulnerability SLAs can be adjusted. Data handling requirements are universal.
Security-sensitive changes **REQUIRE** review from someone with security context. Dependency vulnerability alerts **MUST NOT** be ignored or silently dismissed.

</SpecSection>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Issues estimated larger than **M** (or the team's medium equivalent) **MUST NOT*

- All acceptance criteria in the description are met
- Code is reviewed and merged
- Tests cover the changed behavior (per the [Test Requirements](/sdk/getting-started/standards/code-quality#test-requirements-by-change-type) standard)
- Tests cover the changed behavior (per the [Test Requirements](/sdk/getting-started/standards/code-quality#testing) standard)
- Any user-facing documentation impact is either shipped or tracked in a follow-up issue

**Project level** — a project is done when:
Expand Down
2 changes: 1 addition & 1 deletion develop-docs/sdk/getting-started/standards/review-ci.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Every SDK repo **MUST** have as required status checks:

Optional/**RECOMMENDED**:

- Size/Performance Check ([SDK size and performance budgets](/sdk/getting-started/standards/code-quality#sdk-size-and-performance-budgets))
- Size/Performance Check ([SDK size and performance budgets](/sdk/getting-started/standards/code-quality#size-performance-budgets))
- API Snapshot ([Public API change approval gate](/sdk/getting-started/standards/api-architecture#public-api-change-approval-gate))
- Changelog Check ([Changelog entry](/sdk/getting-started/standards/code-submission#changelog-entry))
- Secret Scanning ([Security practices](/sdk/getting-started/standards/code-quality#security-practices)).
Expand Down
Loading