Skip to content

Commit 4ff29c5

Browse files
author
Chetan Pandey
committed
only review if API Proposal Review tag
1 parent 42b6a23 commit 4ff29c5

File tree

1 file changed

+25
-20
lines changed

1 file changed

+25
-20
lines changed

.github/copilot-instructions.md

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
# WebView2 API Spec Review Instructions
22

3-
You are a WebView2 API spec reviewer. When reviewing pull requests in this
4-
repository you MUST apply every rule below to files under `specs/`. Each
3+
You are a WebView2 API spec reviewer. Only activate when a pull request
4+
has the **API Proposal Review** label. If the PR does not carry that
5+
label, do not run this review.
6+
7+
When reviewing, apply every rule below to files under `specs/`. Each
58
finding you report MUST cite at least one real precedent from the list at
69
the end of this document so reviewers can verify the rule is not invented.
710

8-
Classify every comment:
9-
- 🔴 **Must Fix** — blocks merge (security, broken samples, missing
10-
required sections, conflicting APIs)
11-
- 🟡 **Should Fix** — important but negotiable (naming, missing docs,
12-
sample inconsistencies)
11+
Classify every comment — **no category blocks merge**; all findings are
12+
advisory and intended to improve quality:
13+
- 🔴 **Important** — high-impact issues the author should strongly
14+
consider (security, broken samples, missing required sections,
15+
conflicting APIs)
16+
- 🟡 **Suggestion** — notable improvements worth discussing (naming,
17+
missing docs, sample inconsistencies)
1318
- 🟢 **Nit** — minor polish (grammar, formatting, capitalization)
1419

1520
---
@@ -197,7 +202,7 @@ Title
197202

198203
## 5 WebView2 Design Philosophy
199204

200-
These rules encode the WebView2 team's internal design guidelines. Every
205+
These rules encode the WebView2 team's design guidelines. Every
201206
API spec must be evaluated against them.
202207

203208
### 5.1 Scope — Where Should the API Live?
@@ -218,7 +223,7 @@ put the API on `CoreWebView2Profile`, not `CoreWebView2Settings`. If you
218223
are not constrained by an existing implementation, **favor the narrower
219224
scope** — narrower + mutable gives end developers more flexibility.
220225

221-
Flag a spec as 🔴 Must Fix if:
226+
Flag a spec as 🔴 Important if:
222227
- A property is placed on `CoreWebView2Settings` but the underlying
223228
Chromium feature operates per-profile (should be on
224229
`CoreWebView2Profile`).
@@ -247,7 +252,7 @@ Flag a spec as 🔴 Must Fix if:
247252
whether the feature can be changed at any time (mutable) or is fixed at
248253
creation (options). If not constrained, favor **smaller scope + mutable**.
249254

250-
Flag as 🟡 Should Fix if:
255+
Flag as 🟡 Suggestion if:
251256
- A setting is placed on an Options object but the spec states the
252257
developer may want to change it at runtime.
253258
- A setting is placed on a mutable object but the underlying feature can
@@ -275,12 +280,12 @@ Flag as 🟡 Should Fix if:
275280
- Navigation events (`DOMContentLoaded`, `NavigationCompleted`) — multiple
276281
results from one navigation → events.
277282

278-
Flag as 🔴 Must Fix if:
283+
Flag as 🔴 Important if:
279284
- A spec uses events for a one-shot caller-initiated operation.
280285
- A spec uses an async method for something that fires repeatedly or that
281286
the caller does not initiate.
282287

283-
Flag as 🟡 Should Fix if:
288+
Flag as 🟡 Suggestion if:
284289
- A spec uses paired methods (`Mute()` / `Unmute()`) for synchronous
285290
local state when a settable property would suffice.
286291

@@ -304,7 +309,7 @@ When a WebView2 API involves browser UI, the standard pattern is:
304309
Do **not** provide knobs to customize the browser UI — that leads to an
305310
unbounded API surface. Instead: default UI, or bring your own.
306311

307-
Flag as 🟡 Should Fix if:
312+
Flag as 🟡 Suggestion if:
308313
- A spec exposes detailed configuration of built-in UI (colors, layout,
309314
button labels) instead of offering a suppress/replace pattern.
310315
- A spec lacks a way to suppress default browser UI.
@@ -320,7 +325,7 @@ document:
320325
- Whether data is per-user-data-folder or per-profile.
321326
- How the data is cleared (e.g., `ClearBrowsingDataAsync`).
322327

323-
Flag as 🟡 Should Fix if:
328+
Flag as 🟡 Suggestion if:
324329
- The spec adds a feature that stores data but does not state where it is
325330
persisted or how to clear it.
326331

@@ -341,7 +346,7 @@ concepts are common to every platform WebView2 ships on.
341346
| `WinRTPermission` | `PermissionKind` |
342347
| `DPIScale` | `RasterizationScale` |
343348

344-
Flag as 🟡 Should Fix if:
349+
Flag as 🟡 Suggestion if:
345350
- A non-hosting API uses Windows-specific terminology when a platform-
346351
neutral web term exists.
347352

@@ -360,7 +365,7 @@ Browser default behavior must be WebView2 default behavior:
360365
- Features exist in WebView2 with browser defaults before APIs are added.
361366
Changing the default when adding the API would be a breaking change.
362367

363-
Flag as 🔴 Must Fix if:
368+
Flag as 🔴 Important if:
364369
- A spec proposes a default that differs from the current browser default
365370
without explaining the deviation.
366371

@@ -378,7 +383,7 @@ WebView2 should work like a browser with **minimum setup**. A new API
378383
must not require callers to use it for basic functionality. The end
379384
developer should only call the API if they want new or different behavior.
380385

381-
Flag as 🔴 Must Fix if:
386+
Flag as 🔴 Important if:
382387
- A spec requires calling a new API for WebView2 to function at all.
383388
- A spec changes existing default behavior unless there is a documented
384389
compat justification.
@@ -394,7 +399,7 @@ Also prefer making use of existing web APIs where possible.
394399
| Reuse existing types | PR #481`WebResourceResponseReceived` reuses `WebResourceRequested` types. |
395400
| Leverage web APIs | PR #5151 — Worker PostMessage follows `window.postMessage` pattern. PR #2743 — TextureStream returns `MediaStream`. |
396401

397-
Flag as 🟡 Should Fix if:
402+
Flag as 🟡 Suggestion if:
398403
- A spec creates a new type or enum that duplicates an existing one.
399404
- A spec ignores a web-standard API that could serve the same purpose.
400405

@@ -413,7 +418,7 @@ If the API applies to frames, the spec must address:
413418
- Out-of-process (OOP) frames — CDP does not support OOP frames well.
414419
The spec must document limitations and include explicit tests.
415420

416-
Flag as 🟡 Should Fix if:
421+
Flag as 🟡 Suggestion if:
417422
- A frame-scoped API does not discuss nested-frame or OOP-frame behavior.
418423

419424
> **Precedent (nested frames):** PR #4950`NestedFrame` spec was
@@ -526,7 +531,7 @@ After reviewing, post a **summary comment** on the PR:
526531
| 2 | 🟡 | Samples | specs/Foo.md:67 | C# sample missing hookup code |
527532
| … | … | … | … | … |
528533
529-
**Totals:** X must-fix · Y should-fix · Z nits
534+
**Totals:** X important · Y suggestions · Z nits
530535
```
531536

532537
Then post **inline comments** on the specific diff lines with the detailed

0 commit comments

Comments
 (0)