Skip to content

Add review guidance for deprecated components - #32620

Merged
Edward Chen (edgchen1) merged 2 commits into
mainfrom
edgchen1/deprecation_agent_instructions
Sep 17, 2026
Merged

Edward Chen (edgchen1) merged 2 commits into
mainfrom
edgchen1/deprecation_agent_instructions

Conversation

@edgchen1

Copy link
Copy Markdown
Contributor

Description

Add agent review guidance for deprecated components.

Motivation and Context

Instruct agent reviewer to direct PRs away from updating deprecated components.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 15, 2026 22:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The WebGL scope omits deprecated implementation paths, and JSEP policy duplication risks documentation drift.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds path-scoped review guidance for deprecated components.

Changes:

  • Adds WebGL and JSEP maintenance/removal policies.
  • Adds ACL EP deprecation guidance.
File summaries
File Description
.github/instructions/webgl-deprecated.instructions.md Defines WebGL review policy.
.github/instructions/jsep-deprecated.instructions.md Defines JSEP review policy.
.github/instructions/acl-deprecated.instructions.md Defines ACL EP review policy.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/instructions/webgl-deprecated.instructions.md
Comment thread .github/instructions/jsep-deprecated.instructions.md
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The JSEP scope omits central files controlling its runtime and build integration.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/instructions/jsep-deprecated.instructions.md:3

  • The scope omits central JSEP integration files, so this policy will not load for changes that alter JSEP selection or prolong its build wiring. For example, js/web/script/build.ts:57-84 selects JSEP versus the native WebGPU EP, and cmake/onnxruntime_webassembly.cmake:219-251 adds JSEP exports and pre-jsep.js; both are explicitly part of the migration path but are outside applyTo. Add these files (and the other shared runtime integration files that own JSEP-specific branches) to the scope so such PRs receive this guidance.
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@edgchen1

Copy link
Copy Markdown
Contributor Author
  • The scope omits central JSEP integration files, so this policy will not load for changes that alter JSEP selection or prolong its build wiring. For example, js/web/script/build.ts:57-84 selects JSEP versus the native WebGPU EP, and cmake/onnxruntime_webassembly.cmake:219-251 adds JSEP exports and pre-jsep.js; both are explicitly part of the migration path but are outside applyTo. Add these files (and the other shared runtime integration files that own JSEP-specific branches) to the scope so such PRs receive this guidance.

the intent is to flag new development on JSEP. I think we don't need to include shared files in the applyTo section.

@edgchen1
Edward Chen (edgchen1) merged commit b01a443 into main Sep 17, 2026
88 of 91 checks passed
@edgchen1
Edward Chen (edgchen1) deleted the edgchen1/deprecation_agent_instructions branch September 17, 2026 16:29
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