Skip to content

docs: PR review rules — the version bump becomes a calculation - #73

Merged
MarketDataApp merged 3 commits into
mainfrom
sdk-review-rules
Sep 17, 2026
Merged

MarketDataApp merged 3 commits into
mainfrom
sdk-review-rules

Conversation

@MarketDataApp

Copy link
Copy Markdown
Owner

What

Adds .github/code-review/REVIEW.md: the standing instructions the PR review
bot reads for this repository. The file opens with @include default, so these
rules sit on top of the shipped defaults rather than replacing them.

Generated from docs/sdk/ in MarketData-App/code-review-bot. Edit it there and
regenerate, so the shared part cannot drift across the six SDKs.

The rule this exists for

A non-breaking change takes the next minor, a breaking change takes the next
major, and each SDK decides on its own — the same API change can break
Java's dte(String) and leave Python's dynamic signature untouched.

The reviewer does not form an opinion about whether a change is breaking. It
compares the public surface before and after, reports the table as evidence, and
applies a mechanical test: a public symbol that disappeared or a type that
changed is breaking. The bump becomes a calculation, which is what stops a
major shipping by accident.

Two questions that both get called "breaking" are kept apart: whether the API
change is breaking (one answer for every SDK, from the schema diff, decided
upstream) against whether a pull request breaks this SDK's public surface (the
reviewer's question, and the one that sets the bump).

Also in the file

  • how the bump arrives with a pull request: a CHANGELOG entry under a BREAKING
    heading, a migration line, and the version impact stated in the description
  • what needs no bump at all: tests, CI, formatting, internal refactoring
  • API alignment: a pull request that adds an API capability covers the whole
    capability, not the subset the author needed
  • sibling defects: the candle chunking bug was present in four of the five SDKs,
    so a fix to shared logic says whether the siblings have it
  • one live integration test per endpoint, and a suite that skips on a missing
    token is blocking because it reports success while testing nothing
  • what counts as this language's public surface, and exactly what breaks it

Version impact

none. Documentation for the review bot. No source file changes, no release.

@codecov

codecov Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.27%. Comparing base (845e539) to head (491bfaf).

Additional details and impacted files
@@            Coverage Diff            @@
##               main      #73   +/-   ##
=========================================
  Coverage     98.27%   98.27%           
  Complexity     1043     1043           
=========================================
  Files           123      123           
  Lines          2606     2606           
  Branches        301      301           
=========================================
  Hits           2561     2561           
  Partials         45       45           

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 845e539...491bfaf. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MarketDataApp
MarketDataApp merged commit c1f9d3d into main Sep 17, 2026
7 checks passed
@MarketDataApp
MarketDataApp deleted the sdk-review-rules branch September 17, 2026 00:04
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.

1 participant