Skip to content

docs: seed initial review rules for automated code review#1867

Open
felixweinberger wants to merge 2 commits intomainfrom
fweinberger/review-conventions
Open

docs: seed initial review rules for automated code review#1867
felixweinberger wants to merge 2 commits intomainfrom
fweinberger/review-conventions

Conversation

@felixweinberger
Copy link
Copy Markdown
Contributor

Seed initial review rules for automated code review.

Types of changes

  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines

@felixweinberger felixweinberger requested a review from a team as a code owner April 9, 2026 13:58
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 9, 2026

⚠️ No Changeset found

Latest commit: e8d167a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 9, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@1867

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@1867

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@1867

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@1867

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@1867

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@1867

commit: e8d167a

@felixweinberger felixweinberger force-pushed the fweinberger/review-conventions branch from 6996f08 to 6b0adac Compare April 9, 2026 14:00
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — purely additive documentation file seeding review conventions, no code changes.

Extended reasoning...

Overview

This PR adds a single new file, REVIEW.md, containing review conventions for the repository. No source code, tests, or configuration files are modified.

Security risks

None. This is a documentation-only change.

Level of scrutiny

Minimal. The file contains prose guidelines and a checklist derived from past human review rounds. The rules are internally consistent and well-grounded in PR references.

Other factors

No bugs were found by the bug hunting system. No changeset is needed for a docs-only addition. The content is already serving as the repo_prompt for automated review, so it can be validated in practice immediately.

@felixweinberger felixweinberger force-pushed the fweinberger/review-conventions branch from 6b0adac to 9894555 Compare April 9, 2026 14:05
@felixweinberger felixweinberger force-pushed the fweinberger/review-conventions branch from 9894555 to a6a37ce Compare April 9, 2026 14:06
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — the stale and references I flagged have been corrected.

Extended reasoning...

Overview

Pure documentation change adding REVIEW.md with review conventions, guiding principles, checklist, and a recurring-catches section. No code changes.

Security Risks

None. This is a documentation file with no executable code.

Level of Scrutiny

Low. The only risk is stale or misleading guidance, which was the exact concern in my prior review. Both issues I raised (stale class name and stale Zod v3 API) were fixed in the follow-up commit . The current diff correctly uses / and throughout.

Other Factors

No code, no tests needed, no changeset needed. Content is consistent with the codebase conventions I can verify.

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