ci: sync translations only for PRs merged into main - #846
Conversation
The sync workflows triggered on `pull_request: types: [closed]` with no `branches:` filter, so a PR merged into any branch fired them. A merge into the long-lived `jb2` branch opened translation PRs in every target repo for content the published edition does not carry. Mirrors the template change in QuantEcon/action-translation#308 (v0.28.1). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
✅ Deploy Preview for taupe-gaufre-c4e660 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
🟡 Changes recommended
The new base-branch filter does not apply to the issue_comment/\translate-resync trigger path, so non-main merged PRs can still initiate sync runs in a way that conflicts with the stated goal.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR tightens the translation sync GitHub Actions trigger so that automatic translation-sync runs only happen for PRs merged into the main branch, preventing translation PRs from being opened based on merges into long-lived work/migration branches.
Changes:
- Adds
branches: [main]to thepull_request(closed) trigger for the Simplified Chinese translation sync workflow. - Adds an explanatory comment describing why the base-branch filter is needed.
File summaries
| File | Description |
|---|---|
| .github/workflows/sync-translations-zh-cn.yml | Restricts the PR-merge trigger to base branch main and documents the rationale. |
Review details
Suppressed comments (1)
.github/workflows/sync-translations-zh-cn.yml:16
- The new
branches: [main]filter only applies to thepull_requesttrigger. The workflow can still be triggered viaissue_comment(\translate-resync) on a PR merged into a non-mainbase branch, which can still open translation PRs in target repos (especially whileQuantEcon/action-translation@v0does not yet include the base-branch refusal mentioned in the PR description). To fully meet the goal of “sync only for PRs merged into main”, add an equivalent base-branch gate for theissue_commentpath (e.g., fetch the PR via the GitHub API and only run the sync step whenbase.ref == 'main'), or temporarily disable the resync comment trigger until@v0carries v0.28.1+.
branches: [main]
paths:
- 'lectures/**/*.md'
- 'lectures/_toc.yml'
issue_comment:
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Only merges into main sync. Without this, a PR merged into any branch | ||
| # (a long-lived migration branch, say) opens translation PRs for content | ||
| # the published edition does not carry. |
There was a problem hiding this comment.
Fixed in b727281 — the comment now reads "Only PRs merged into main trigger a sync." followed by the same explanation. Same reword applied to the sibling PR QuantEcon/lecture-python.myst#1056.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Adds
branches: [main]to thepull_requesttrigger of every translation-sync workflow in this repo.Why.
pull_request: types: [closed]fires for a PR closed against any base branch, and the job'smerged == truecheck does not look at the base either. On 2026-09-12 a theme migration merged into the long-livedjb2branch fired the sync workflows and opened translation PRs in the target repos for content the published edition does not carry (those PRs have been closed). A merge into a work branch is not a publication event.What changes. One line (plus a comment) per workflow: the trigger now requires the PR's base to be
main. The\translate-resynccomment path is unchanged here; the action itself now refuses a resync on a PR whose base is not the default branch, as of QuantEcon/action-translation v0.28.1 (see QuantEcon/action-translation#308). Until@v0carries that release, this filter is the only guard, which is why it lands now rather than waiting.When
jb2itself merges intomain, that PR will trigger a sync that carries its accumulated changes in one run, as intended.🤖 Generated with Claude Code