diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md deleted file mode 100644 index 9039fd4841..0000000000 --- a/.github/skills/code-review/SKILL.md +++ /dev/null @@ -1,108 +0,0 @@ ---- -name: code-review -description: >- - Review pull requests in the Temporal documentation repository against AGENTS.md - and docs style conventions. Use when reviewing docs, MDX, sidebar, redirect, or - Docusaurus changes in temporalio/documentation. ---- - -# Temporal documentation code review - -Review this PR as a Temporal docs change. Prefer concrete, actionable comments. -Cite paths when asking for a fix. Do not nitpick optional style that Vale does not -enforce in CI unless it hurts clarity or correctness. - -## Read these first (when relevant to the diff) - -| Topic | File | -| ----- | ---- | -| Agent / repo guidance | [AGENTS.md](../../../AGENTS.md) | -| Where content belongs | [readme/INFORMATION-ARCHITECTURE.md](../../../readme/INFORMATION-ARCHITECTURE.md) | -| Style | [readme/STYLE.md](../../../readme/STYLE.md) | -| React components in MDX | [readme/COMPONENTS.md](../../../readme/COMPONENTS.md) | -| Component → Markdown mapping | [readme/COMPONENT_REGISTRY.md](../../../readme/COMPONENT_REGISTRY.md) | -| Mermaid | [readme/MERMAID.md](../../../readme/MERMAID.md) | -| LLM Markdown pipeline | [MARKDOWN_PIPELINE.md](../../../MARKDOWN_PIPELINE.md) | -| CI / automations | [readme/AUTOMATIONS.md](../../../readme/AUTOMATIONS.md) | - -## Must-check for every docs PR - -### Correctness and scope - -- Claims match current product behavior (no future promises unless tied to a version or release stage). -- New or moved pages land in the right IA section. -- Changing `id` or `slug` includes a `vercel.json` redirect plan. -- Adding or moving pages updates `sidebars.js` and fixes inbound links when needed. -- Draft PRs are appropriate when blocked on upstream/SDK/other-team work (see AGENTS.md Pull requests). - -### Temporal terminology (flag these) - -- Capitalize Temporal core terms as proper nouns (Workflow, Activity, Worker, Namespace, etc.). -- Prefer **Temporal Service** over **Cluster** / **Temporal Cluster** in body prose (exceptions: Multi-Cluster Replication, code/config identifiers, CLI names). -- Outside core terms, spell out "identifier." For core terms use `Id` (not `ID` / `id`), e.g. Workflow Id. -- Prefer **use** over utilize/leverage; avoid filler and vague intensifiers (powerful, robust, seamless, unlock, streamline). - -### Headings and frontmatter - -- Headings: sentence case; prefer infinitive or questions; Develop pages use task-oriented headings. -- Frontmatter: clear one-sentence `description`; match `tags` to siblings; do **not** add unused `keywords`. -- Do not invent new tags/keywords unless this is a genuinely new feature area. - -### MDX and components - -- Prefer shared imports from `@site/src/components`. -- Reuse existing components (`Tabs`, `SdkTabs`, `CaptionedImage`, etc.) instead of one-off HTML. -- New public components should be exported from `src/components/index.js` when needed. -- Check COMPONENTS.md / COMPONENT_REGISTRY.md before blessing a new pattern. - -### Code samples - -- Prefer Snipsync from CI-enabled sample repos when a matching snippet exists. -- Snip edits belong in the **source** sample repo, then `yarn snipsync`. - -### Style / CI bar - -- CI-scoped Vale is the merge bar: `vale --config .vale-ci.ini` on touched files (Headings + RelativeLinks). -- Do not require fixing the full Vale suggestion set unless the author opted into it. -- Relative internal links preferred where Vale RelativeLinks applies. - -## Diff-type checklists - -### New or heavily rewritten MDX page - -- [ ] IA placement and sidebar entry -- [ ] Redirects if URL changed -- [ ] Terminology + headings + description -- [ ] Components reused appropriately -- [ ] Samples: Snipsync or justified docs-owned code - -### `sidebars.js` / navigation only - -- [ ] IDs resolve to real pages -- [ ] Nesting and labels match section conventions - -### `vercel.json` redirects - -- [ ] Old public paths covered -- [ ] Destinations are current canonical URLs - -### UI / React (`src/components`, theme, CSS) - -- [ ] Fits existing design system; no one-off layout inventions without need -- [ ] Accessibility basics (labels, contrast) when relevant -- [ ] Public exports updated if a new shared component is introduced - -### Config / build (`docusaurus.config.js`, plugins) - -- [ ] Internal footer/`to:` links are real routes (plugin-generated files like `/llms.txt` need absolute URLs or they break `onBrokenLinks: 'throw'`) - -## How to leave comments - -- Group related nits; lead with blockers (wrong term, broken URL, missing redirect/sidebar). -- Separate **blocking** vs **suggestion**. -- If unsure about product behavior, ask rather than inventing. - -## Out of scope for this skill - -- Rewriting the entire PR in review comments -- Enforcing full Vale / Google style beyond AGENTS.md and CI-scoped rules diff --git a/AGENTS.md b/AGENTS.md index 7b083c65b0..54ef8b8e97 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -210,3 +210,104 @@ noisier suggestion-level rules that are not enforced in CI and are not a require `.vale-ci.ini` enables only the small set of high-confidence rules (`Temporal.Headings`, `Temporal.RelativeLinks`) used as a CI gate in `.github/workflows/vale-ci.yml`. `.vale.ini` (the default config) runs the full style set, which includes noisier suggestion-level rules not enforced in CI. + +## Code Review Rules + +Guidance for automated reviewers on pull requests. Review a pull request when it changes files under +`docs/`. Check the three things below, and nothing else. + +Do not review: + +- Dependabot pull requests, and lockfile-only changes. +- Pull requests that change no files under `docs/`. +- Pull requests opened by the `temporal-cicd` app. These regenerate content from a source of truth + outside this repository: snipsync snippets, the CLI command reference, the Cloud permissions table, + and SDK version chips. Nothing in them can be fixed here. +- Reverts. +- Pull requests from authors outside the `temporalio` organization, unless a maintainer asks for a + review on the pull request. + +When a pull request is out of scope, stop without commenting. + +### 1. Collateral changes + +The same fact usually appears on several pages, and a pull request that changes it in one place often +misses the others. + +- Search the rest of `docs/` for pages that state the same fact, and name the ones that should + probably change too. Give paths. +- When a page documents something belonging to a larger feature, check whether the parent, summary, + and index pages for that feature need the same addition. + +Name specific files. A general reminder to check other pages is not a finding. + +### 2. Structural fit + +- Does a new or moved page belong in the section it was put in? Use + [INFORMATION-ARCHITECTURE.md](./readme/INFORMATION-ARCHITECTURE.md). +- Does a large part of the page cover content that belongs on another page or in another section? +- Is the page long enough to split? Aim for fewer than 10 top-level headings and fewer than 15 + headings in total. +- Do the frontmatter `tags` match the sibling pages in the same section? + +### 3. Text tone + +Flag the writing, never the author. Say nothing about who wrote the text or how it was produced. + +Generated register: + +- Long sentences carrying little information, such as "That distinction matters because ...". +- Software jargon used as casual speech: "cross-cutting", "load-bearing". +- "Quietly" or similar words added to a sentence that does not need them. +- Junk drawer lists, meaning bullets collected under one heading with no shared idea holding them + together. + +Marketing register: + +- Opening a page with a leading question. +- Repeated, eager mentions of paid features where they are not the subject. +- Vague intensifiers standing in for a fact. See [Writing style](#writing-style) above. + +Pages under `docs/evaluate/` are allowed some marketing register. Hold reference, develop, and +production-deployment pages to a stricter line. + +Before. Two rhetorical questions, then a paragraph restating the premise, and the actual capability +arrives last: + +> Temporal keeps your Workflows running even when a Worker crashes. But what happens when a whole +> data center crashes? Or a region? +> +> In the cloud, outages are commonplace. An outage can bring down a whole data center, cluster, +> region, or cloud provider. To be durable in the cloud, Workflows and applications must handle +> these outages smoothly, just like Temporal handles a Worker crash. +> +> Temporal Cloud's High Availability features add extra reliability to Temporal Cloud Namespaces by +> handling cloud outages. Using asynchronous replication between multiple regions or cloud +> providers, combined with automatic outage detection and failover, High Availability keeps your +> Workflows running even during a cloud region outage. + +After. Same facts, leading with what the feature does: + +> Temporal Cloud High Availability uses asynchronous replication between multiple regions or cloud +> providers, with automatic outage detection and failover, to keep your Workflows running during a +> cloud region outage. + +### What not to flag + +- Anything CI already reports. Redirects, orphaned pages, broken links, build failures, Mermaid + syntax, and the Vale rules in `.vale-ci.ini` all run on every pull request. See + [AUTOMATIONS.md](./readme/AUTOMATIONS.md). +- Terminology, capitalization, and word choice. Vale owns these. Rules that Vale does not enforce + today get added to Vale, not to this review. +- Content inside `` and `` blocks. That code comes from a sample + repository and cannot be fixed here. A snippet that disappeared, or prose that no longer matches + the snippet it describes, is still worth flagging. +- Product behavior you cannot confirm from this repository. Ask instead of guessing. + +### How to leave comments + +- Anchor a finding to a line when it has one. Put the rest, such as a page in the wrong section, in + the review body. +- One finding per comment, with the path and the fix you are suggesting. +- State the finding and the fix. Do not say whether it should block the merge. +- Group nits rather than posting each one separately, and keep them few.