Skip to content

Improved i18n CI install time#28018

Merged
9larsons merged 1 commit into
TryGhost:mainfrom
JohnONolan:codex/speed-up-i18n-ci
May 21, 2026
Merged

Improved i18n CI install time#28018
9larsons merged 1 commit into
TryGhost:mainfrom
JohnONolan:codex/speed-up-i18n-ci

Conversation

@JohnONolan
Copy link
Copy Markdown
Member

What changed

  • Skips lifecycle scripts in the setup job dependency install.
  • Filters the i18n job install to @tryghost/i18n... so it does not install the full monorepo.
  • Runs the i18n package test directly with pnpm --filter @tryghost/i18n test.

Why

The i18n job only needs the i18n workspace dependency graph, but CI was doing a full monorepo install before running this isolated check. Local act validation showed the job dropping from 243.80s to 159.84s for the same workflow job, with the filtered i18n install step taking 14.2s.

Testing

  • git diff --check -- .github/workflows/ci.yml
  • act push -W .github/workflows/ci.yml -j job_i18n --eventpath /tmp/act-ci-baseline-push.json --container-architecture linux/amd64 --pull=false --action-offline-mode --no-cache-server

Filtered the i18n job dependency install to the @tryghost/i18n workspace so CI does not install the full monorepo for this isolated check.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Review Change Stack

Walkthrough

The CI workflow was updated to suppress package installation lifecycle scripts across both the main Setup job and the i18n job by adding the --ignore-scripts flag. The i18n job was additionally refactored to filter dependency installation to only the @tryghost/i18n workspace and changed test execution from using the Nx target runner to pnpm's filter-based approach.

Suggested reviewers

  • 9larsons
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Improved i18n CI install time' directly summarizes the main change: optimizing CI workflow installation time for the i18n job through dependency filtering and script skipping.
Description check ✅ Passed The description clearly explains what changed, why the changes were made, and includes specific testing steps, all directly related to the CI workflow optimization outlined in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

289-308: 💤 Low value

Consider adding explicit permissions block to follow least-privilege principle.

The static analysis tool flags this job for using default permissions. Since the i18n job only performs read operations (checkout, install, test), you could add an explicit permissions block to follow the principle of least privilege:

job_i18n:
  runs-on: ubuntu-latest
  needs: [job_setup]
  name: i18n
  permissions:
    contents: read
  if: |
    needs.job_setup.outputs.is_tag == 'true'
    || needs.job_setup.outputs.changed_i18n_apps == 'true'

This makes the job's required permissions explicit and prevents potential over-privileged execution. Based on learnings, other jobs in this workflow already follow this pattern where appropriate.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 289 - 308, The job_i18n workflow
currently relies on default GitHub Actions permissions; add an explicit
permissions block to restrict its access (e.g., set contents: read) so
checkout/install/test steps (actions/checkout, pnpm/action-setup,
actions/setup-node and the pnpm commands) run with least privilege; locate the
job definition named job_i18n and insert a permissions: section directly under
the job header to explicitly grant only the required read permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 289-308: The job_i18n workflow currently relies on default GitHub
Actions permissions; add an explicit permissions block to restrict its access
(e.g., set contents: read) so checkout/install/test steps (actions/checkout,
pnpm/action-setup, actions/setup-node and the pnpm commands) run with least
privilege; locate the job definition named job_i18n and insert a permissions:
section directly under the job header to explicitly grant only the required read
permissions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5469772c-a02e-4e0a-bee4-d7b3e672cd94

📥 Commits

Reviewing files that changed from the base of the PR and between bd46a1c and 20506cf.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.83%. Comparing base (bd46a1c) to head (20506cf).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #28018   +/-   ##
=======================================
  Coverage   73.83%   73.83%           
=======================================
  Files        1523     1523           
  Lines      128982   128982           
  Branches    15483    15483           
=======================================
  Hits        95233    95233           
- Misses      32787    32811   +24     
+ Partials      962      938   -24     
Flag Coverage Δ
admin-tests 53.57% <ø> (ø)
e2e-tests 73.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JohnONolan JohnONolan requested a review from 9larsons May 21, 2026 07:08
@9larsons 9larsons merged commit 4daaa5b into TryGhost:main May 21, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants