[docs] docs: unbloat triggers.md (1040 → 810 lines, 22% reduction)#35015
Conversation
Remove duplicate documentation of `on.steps:` (was covered in both "Filtering by Custom Steps" and "Pre-Activation Steps"). Consolidate skip-if-match / skip-if-no-match auth/scope notes by cross-referencing instead of restating. Condense bullet lists (fork specifications, supported input types, common bot names, workflow_run protections) into prose. Merge near-duplicate roles/bots example blocks. Remove redundant code samples where the configuration is already shown in a prior block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧪 Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. The PR '[docs] docs: unbloat triggers.md (1040 → 810 lines, 22% reduction)' (#35015) only modifies docs/src/content/docs/reference/triggers.md (documentation). Test Quality Sentinel skipped. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #35015 does not have the 'implementation' label and has 0 new lines of code in default business logic directories (≤100 threshold). |
There was a problem hiding this comment.
Pull request overview
This PR streamlines the Triggers reference documentation by removing duplicated explanations and consolidating repetitive examples/prose while aiming to preserve all technical semantics and links.
Changes:
- Condensed repeated sections (e.g., fuzzy scheduling, skip-if behavior notes, workflow_run protections) into shorter prose and merged example blocks.
- Replaced several full-workflow examples with focused snippets or cross-references to authoritative sections.
- Simplified/merged YAML examples for roles/bots filtering and removed redundant scaffolding in markdown examples.
Show a summary per file
| File | Description |
|---|---|
| docs/src/content/docs/reference/triggers.md | Removes duplicated trigger/filtering documentation and consolidates examples to reduce length while keeping the same documented behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
docs/src/content/docs/reference/triggers.md:623
- This section’s
github-appexample usesclient-id, but a few lines below the table still saysgithub-apprequiresapp-id. Please align the wording here (and the table) to documentclient-idas the preferred key, withapp-idonly as a deprecated alias if applicable.
A pre-activation check runs the query against the current repository. If matches reach or exceed the threshold (default `max: 1`), the workflow is skipped. All standard GitHub search qualifiers are supported (`is:`, `label:`, `in:title`, `author:`, etc.).
Use `scope: none` to remove the automatic repo qualifier and search org-wide. For cross-repo or org-wide searches that need elevated permissions, configure `github-token` or `github-app` at the top-level `on:` section — the same token is shared across all skip-if checks and the activation job:
docs/src/content/docs/reference/triggers.md:690
- The explicit-output example references
steps.label_check.outputs.has_bug_label, but the preceding step example (above) doesn’t show setting that output (it only relies on exit codes). Consider adding a minimal step snippet that writeshas_bug_labelto$GITHUB_OUTPUT, or explicitly state where that output is defined, so copy/paste readers don’t end up with a non-working example.
To pass an explicit value rather than relying on exit codes, set a step output (e.g., `echo "has_bug_label=true" >> "$GITHUB_OUTPUT"`) and re-expose it via `jobs.pre-activation.outputs`:
```yaml wrap
jobs:
pre-activation:
outputs:
has_bug_label: ${{ steps.label_check.outputs.has_bug_label }}
- Files reviewed: 1/1 changed files
- Comments generated: 3
| ``` | ||
|
|
||
| The `github-app` object accepts the same fields as the GitHub App configuration used elsewhere in the framework (`app-id`, `private-key`, and optionally `owner` and `repositories`). The token is minted once in the pre-activation job and is shared across the reaction step, the status comment step (if `status-comment: true`), and any skip-if search steps. | ||
| The `github-app` object accepts `app-id`, `private-key`, and optionally `owner` and `repositories` — the same fields used elsewhere in the framework. The token is minted once in the pre-activation job. |
| The `github-app` object accepts `app-id`, `private-key`, and optionally `owner` and `repositories` — the same fields used elsewhere in the framework. The token is minted once in the pre-activation job. | ||
|
|
||
| Both `github-token` and `github-app` can be defined in a **shared agentic workflow** and will be automatically inherited by any workflow that imports it (first-wins strategy). This means a central CentralRepoOps shared workflow can define the app config once and all importing workflows benefit automatically: | ||
| Both fields can be defined in a **shared agentic workflow** and are inherited by importers (first-wins). A central CentralRepoOps shared workflow can define the app config once and all importers benefit: |
| ``` | ||
| For conditions based on GitHub search results, use [`skip-if-match:`](#skip-if-match-condition-skip-if-match) or [`skip-if-no-match:`](#skip-if-no-match-condition-skip-if-no-match) in the `on:` section. These accept standard [GitHub search query syntax](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests) and produce the same skipped-not-failed behavior. | ||
|
|
||
| ### Filtering by Repository Access Roles (`on.roles:`, `on.skip-roles`) |
There was a problem hiding this comment.
Documentation cleanup — technically accurate, well-structured reduction. One non-blocking observation.
### Review summary
What's good: All condensations are technically accurate. Cross-references resolve. No YAML field names, valid values, or behavioral semantics were altered. The fuzzy scheduling, roles/bots/author-association, and lock-for-agent sections are all cleaner without losing correctness.
One observation: The state: filtering section previously had a NOTE block showing the exact generated if: expression (github.event_name != 'deployment_status' || ...). This was the most concrete diagnostic aid in that section — users who hit unexpected skip behavior and inspect compiled YAML would know exactly what to look for. The replacement prose ("guard ensures dispatch passes through") is accurate but opaque. Restoring just the expression as a compact blockquote would recapture the debugging value at minimal line cost.
All other removals are unambiguous improvements.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 1.1M
| workflow_dispatch: # Safely combined — guard ensures dispatch passes through | ||
| ``` | ||
|
|
||
| Valid `state` values: `error`, `failure`, `pending`, `success`, `inactive`, `in_progress`, `queued`, `waiting`. |
There was a problem hiding this comment.
The removed NOTE contained the exact generated if: expression, which is the most practical debugging aid for users who want to understand why a combined trigger behaves differently than expected.
💡 Suggestion
Consider keeping a compact inline reference to the generated condition:
> The compiler emits: `github.event_name != 'deployment_status' || (github.event.deployment_status.state == 'failure')`Without it, "guard ensures dispatch passes through" is opaque to users who hit unexpected skips and inspect the compiled YAML trying to understand the behavior.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /grill-with-docs — no blocking issues; 3 minor observations on condensation nuance.
📋 Key Themes & Highlights
Positive Highlights
- ✅ 22% line reduction is well-motivated — the removed content was genuinely duplicated or redundant
- ✅ All YAML field names, valid values, and configuration semantics preserved
- ✅ Cross-references (anchors) correctly point to authoritative sections rather than repeating content
- ✅ PR description is thorough and explains every removal category — easy to audit
- ✅ The inline code example for
echo "has_bug_label=true" >> "$GITHUB_OUTPUT"added in the Pre-Activation Steps section is a net improvement
Minor Observations (inline)
- Fuzzy scheduling block — commented-out alternatives could be misread as multi-value config
status-commentsuppression — "omit or set to false" guidance was droppedgithub-apptoken scope — detail about what steps share the minted token was removed; helpful for permission debugging
All three are suggestions only — none block merging.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.5M
| ```yaml wrap | ||
| on: | ||
| schedule: daily between 9:00 and 17:00 # Scatters within 9am-5pm range | ||
| schedule: daily # Compiler picks a scattered time |
There was a problem hiding this comment.
[/grill-with-docs] The merged fuzzy-scheduling block uses commented-out alternatives inside a single YAML example, which could be misread as a multi-value on: config rather than three mutually-exclusive forms.
💡 Suggestion
A small clarifying label (e.g. # OR: prefix or a trailing note "choose one") would prevent copy-paste errors:
on:
schedule: daily # Compiler picks a scattered time
# OR: schedule: daily around 14:00 # ±1 hour around 2pm
# OR: schedule: daily between 9:00 and 17:00 # Scatters within 9am-5pmMinor nit — the current form is still readable given the prose above it.
| For all other trigger types, `status-comment` must be explicitly set to `true` to enable it. To suppress status comments, omit `status-comment:` or set it to `false`. | ||
|
|
||
| Use an object to enable status comments while selectively disabling specific targets. The object form implies status comments are enabled; each field defaults to `true`: | ||
| For all other triggers, `status-comment` must be explicitly set to `true`. Use an object to selectively disable specific targets (each field defaults to `true`): |
There was a problem hiding this comment.
[/grill-with-docs] The condensed paragraph omits the original guidance that status-comment can also be suppressed by omitting the field (not only by setting it to false). This is a minor but useful clarification for users who want to be explicit.
💡 Optional addition
Consider appending: "To suppress it, omit status-comment: or set it to false."
This was present in the pre-refactor text and is a common question when users see status-comment for the first time.
| ``` | ||
|
|
||
| The `github-app` object accepts the same fields as the GitHub App configuration used elsewhere in the framework (`app-id`, `private-key`, and optionally `owner` and `repositories`). The token is minted once in the pre-activation job and is shared across the reaction step, the status comment step (if `status-comment: true`), and any skip-if search steps. | ||
| The `github-app` object accepts `app-id`, `private-key`, and optionally `owner` and `repositories` — the same fields used elsewhere in the framework. The token is minted once in the pre-activation job. |
There was a problem hiding this comment.
[/grill-with-docs] The condensation drops the detail about what the minted token is shared across (reaction step, status-comment step, skip-if search steps). This is useful context for users troubleshooting permission errors on those sub-steps.
💡 Consider restoring the scope detail
The previous text was:
The token is minted once in the pre-activation job and is shared across the reaction step, the status comment step (if
status-comment: true), and any skip-if search steps.
This is the key "why should I care" for users who set github-app: specifically to get elevated skip-if search permissions.
|
@copilot review all comments and review comments |
|
\n\n@copilot review all comments and address the 7 open review threads, then re-request review once they are resolved.
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all three review comments in commit
|
Summary
Editorial pass on
docs/src/content/docs/reference/triggers.mdreducing file length from ~1040 to ~810 lines (22% reduction) without removing documented functionality.Changes
File modified
docs/src/content/docs/reference/triggers.mdContent removed
aw wrapexamples that duplicated configuration already shown elsewhere (e.g., thelocked-issue-processor.mdworkflow underlock-for-agent:, thesmart-responder.mdand multi-step label-check workflows underon.steps:)statevalues callout) condensed to inline proseNOTEcallout explainingstate:compilation to a GitHub Actionsif:expressionskip-if-no-matchexample that mirroredskip-if-matchusage withgithub-appContent revised
workflow_dispatchinputs section: removed full workflow wrapper, kept only the markdown snippet showing${{ github.event.inputs.topic }}expressionschedule:code blocks into one block with commented variantsdeployment_statusstate:section: merged two single-state / multi-state examples into oneon.steps:section: collapsed to a single forward reference to the Pre-Activation Steps anchor; full examples retained in that dedicated sub-sectionskip-if-no-matchsection: removed embeddedgithub-appexample, now cross-referencesskip-if-matchbehaviouron.roles:/on.skip-roles:section: heading corrected fromon.skip-roles→on.skip-roles:(trailing colon added for consistency)github-appobject documentation: clarified thatclient-idis the canonical field andapp-idis a deprecated aliasCentralRepoOpsshared workflow example: tightened prose, removed duplication between body text and code commentNo functional changes
No configuration keys, default values, compiler behaviour, or cross-links were altered. All documented features remain described.
Commits
ac923c153docs: unbloat triggers.md (1040 → 810 lines, 22% reduction)ab7dd8778docs: fix review feedback in triggers.md