Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the docs site build GitHub Action to be triggered by successful completion of the Validate workflow instead of direct pushes to docs-related files on main, and adds a safeguard so the site build is only triggered when the upstream workflow succeeds. Sequence diagram for updated docs site build trigger workflowsequenceDiagram
actor Developer
participant GitHub
participant Validate_workflow
participant Trigger_Site_Build_workflow
participant Docs_Main_Repo
Developer->>GitHub: Push to main
GitHub->>Validate_workflow: Trigger Validate workflow
Validate_workflow-->>GitHub: Run completes (success or failure)
GitHub->>Trigger_Site_Build_workflow: workflow_run event (Validate completed)
Trigger_Site_Build_workflow->>Trigger_Site_Build_workflow: Check github.event.workflow_run.conclusion == success
alt Validate success
Trigger_Site_Build_workflow->>Docs_Main_Repo: repository-dispatch (Trigger build)
else Validate failure
Trigger_Site_Build_workflow-->>Docs_Main_Repo: No action
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
branchesfilter is now at the same level asworkflow_runinstead of nested under it, so it will be ignored; if you intend to restrict tomain, indentbranchesunderworkflow_runin the YAML. - By removing the
pathsfilter fordocs/**andmkdocs.yml, this workflow will now trigger for any successfulValidaterun (not just docs-related changes); if that’s unintended, consider reintroducing equivalent filtering via conditions ongithub.event.workflow_run.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `branches` filter is now at the same level as `workflow_run` instead of nested under it, so it will be ignored; if you intend to restrict to `main`, indent `branches` under `workflow_run` in the YAML.
- By removing the `paths` filter for `docs/**` and `mkdocs.yml`, this workflow will now trigger for any successful `Validate` run (not just docs-related changes); if that’s unintended, consider reintroducing equivalent filtering via conditions on `github.event.workflow_run`.
## Individual Comments
### Comment 1
<location> `.github/workflows/trigger-site-build.yml:15` </location>
<code_context>
runs-on: ubuntu-latest
steps:
- name: Trigger build in main repo
+ if: ${{ github.event.workflow_run.conclusion == 'success' }}
uses: peter-evans/repository-dispatch@v3
with:
</code_context>
<issue_to_address>
**suggestion (performance):** Consider moving the `if` condition to the job level to avoid starting a runner unnecessarily.
As written, the runner is still provisioned even when `Validate` fails, and only this step is skipped. Applying `if: ${{ github.event.workflow_run.conclusion == 'success' }}` at the `trigger` job level will skip the whole job in that case, reducing runner usage and making the workflow’s behavior clearer.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Trigger build in main repo | ||
| if: ${{ github.event.workflow_run.conclusion == 'success' }} |
There was a problem hiding this comment.
suggestion (performance): Consider moving the if condition to the job level to avoid starting a runner unnecessarily.
As written, the runner is still provisioned even when Validate fails, and only this step is skipped. Applying if: ${{ github.event.workflow_run.conclusion == 'success' }} at the trigger job level will skip the whole job in that case, reducing runner usage and making the workflow’s behavior clearer.
Pull request
Purpose
Describe the problem or feature in addition to a link to the issues.
Approach
How does this change address the problem?
Open Questions and Pre-Merge TODOs
Check all boxes as they are completed
Learning
Describe the research stage
Links to blog posts, patterns, libraries or addons used to solve this problem
Requirements
Check all boxes as they are completed
Summary by Sourcery
CI: