Cleaned up public app release scripts#29414
Conversation
WalkthroughPublic app automation now uses ESM entrypoints and shared metadata utilities. CDN purge matrices derive URLs from Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:test:acceptance |
❌ Failed | 5m 17s | View ↗ |
nx run-many -t test:unit -p @tryghost/adapter-b... |
❌ Failed | 2s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 47s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 4s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 30s | View ↗ |
nx run @tryghost/koenig-lexical:test:acceptance |
✅ Succeeded | 2m 18s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 1m 53s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 43s | View ↗ |
Additional runs (8) |
✅ Succeeded | ... | View ↗ |
💡 Dealing with memory or CPU issues? See memory and CPU details with the resource usage add-on ↗.
☁️ Nx Cloud last updated this comment at 2026-07-17 02:31:29 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@scripts/check-app-version-bump.js`:
- Line 171: Replace the import.meta.main guard in the script entrypoint with a
Node.js-version-compatible check so the app-version consistency validation
always runs when executed directly, including on versions before 21.2.0. Keep
the existing check logic unchanged and avoid relying on import.meta.main.
In `@scripts/release-apps.js`:
- Around line 77-83: Update updatePackageJson to pass 4 as the JSON.stringify
indentation argument when writing package.json, preserving the repository’s
existing formatting and avoiding unrelated reformatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: beaafa74-da4e-43aa-b038-739eb01f93d4
📒 Files selected for processing (17)
.github/workflows/ci.ymlapps/admin-toolbar/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/signup-form/package.jsonapps/sodo-search/package.jsonscripts/README.mdscripts/build-public-apps-matrix.cjsscripts/build-public-apps-matrix.jsscripts/check-app-version-bump.jsscripts/lib/public-apps.jsscripts/public-apps.jsonscripts/release-apps.cjsscripts/release-apps.jsscripts/test/build-public-apps-matrix.test.jsscripts/test/public-apps.test.js
💤 Files with no reviewable changes (2)
- scripts/build-public-apps-matrix.cjs
- scripts/release-apps.cjs
| async function updatePackageJson(newVersion) { | ||
| const newPackageJson = Object.assign({}, packageJson, { | ||
| version: newVersion | ||
| }); | ||
|
|
||
| await writeFile(packageJsonPath, JSON.stringify(newPackageJson, null, 2) + '\n'); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Preserve the existing 4-space indentation in package.json.
The package.json files in this repository (e.g., apps/announcement-bar/package.json) use 4-space indentation. Hardcoding 2 spaces in JSON.stringify will reformat the entire file, creating unnecessary formatting churn and a noisy diff in the release commit. Update this to use 4 spaces to match the repository's convention.
♻️ Proposed fix
async function updatePackageJson(newVersion) {
const newPackageJson = Object.assign({}, packageJson, {
version: newVersion
});
- await writeFile(packageJsonPath, JSON.stringify(newPackageJson, null, 2) + '\n');
+ await writeFile(packageJsonPath, JSON.stringify(newPackageJson, null, 4) + '\n');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function updatePackageJson(newVersion) { | |
| const newPackageJson = Object.assign({}, packageJson, { | |
| version: newVersion | |
| }); | |
| await writeFile(packageJsonPath, JSON.stringify(newPackageJson, null, 2) + '\n'); | |
| } | |
| async function updatePackageJson(newVersion) { | |
| const newPackageJson = Object.assign({}, packageJson, { | |
| version: newVersion | |
| }); | |
| await writeFile(packageJsonPath, JSON.stringify(newPackageJson, null, 4) + '\n'); | |
| } |
🤖 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 `@scripts/release-apps.js` around lines 77 - 83, Update updatePackageJson to
pass 4 as the JSON.stringify indentation argument when writing package.json,
preserving the repository’s existing formatting and avoiding unrelated
reformatting.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29414 +/- ##
=======================================
Coverage 74.17% 74.17%
=======================================
Files 1592 1592
Lines 139000 139000
Branches 16863 16861 -2
=======================================
Hits 103104 103104
- Misses 34845 34877 +32
+ Partials 1051 1019 -32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ref https://linear.app/ghost/issue/PLA-235/consolidate-ci-scripts-into-monorepo-package - convert public app-related scripts to esm - simplify public-apps and infer cdn paths from defaults.json - dedupe code - remove unnecessary changelog generation for public app commits
0b47d92 to
b8db993
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/check-app-version-bump.js (1)
154-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid redundant file reads by extracting variables.
Currently,
getPrVersionandgetPrDefaultsVersionare called twice per app (once for the consistency check and once for the console log). Each call synchronously reads and parsespackage.jsonordefaults.json. Storing the results in local variables improves readability and avoids redundant I/O operations.♻️ Proposed refactor
- const error = checkAppConsistency(app, getPrVersion(app), getPrDefaultsVersion(app)); + const prVersion = getPrVersion(app); + const prDefaultsVersion = getPrDefaultsVersion(app); + const error = checkAppConsistency(app, prVersion, prDefaultsVersion); if (error) { failedApps.push(error); continue; } - console.log(`${app.configKey} version consistency check passed (package.json ${majorMinor(getPrVersion(app))} = defaults.json ${getPrDefaultsVersion(app)})`); + console.log(`${app.configKey} version consistency check passed (package.json ${majorMinor(prVersion)} = defaults.json ${prDefaultsVersion})`);🤖 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 `@scripts/check-app-version-bump.js` around lines 154 - 161, In the app consistency-check loop, update the flow around checkAppConsistency to store getPrVersion(app) and getPrDefaultsVersion(app) results in local variables, then reuse those variables for both the consistency check and success log. Preserve the existing error handling and log output.
🤖 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 `@scripts/check-app-version-bump.js`:
- Around line 154-161: In the app consistency-check loop, update the flow around
checkAppConsistency to store getPrVersion(app) and getPrDefaultsVersion(app)
results in local variables, then reuse those variables for both the consistency
check and success log. Preserve the existing error handling and log output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61a8373f-ca53-4d78-8e5b-60a346aef5c2
📒 Files selected for processing (20)
.github/workflows/ci.ymlapps/admin-toolbar/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/signup-form/package.jsonapps/sodo-search/package.jsonscripts/README.mdscripts/build-public-apps-matrix.cjsscripts/build-public-apps-matrix.jsscripts/check-app-version-bump.jsscripts/lib/public-apps.jsscripts/lib/release-notes.jsscripts/lib/resolve-base-tag.jsscripts/lib/utils.jsscripts/public-apps.jsonscripts/release-apps.cjsscripts/release-apps.jsscripts/test/build-public-apps-matrix.test.jsscripts/test/public-apps.test.js
💤 Files with no reviewable changes (2)
- scripts/build-public-apps-matrix.cjs
- scripts/release-apps.cjs
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/signup-form/package.json
- apps/sodo-search/package.json
- apps/comments-ui/package.json
- apps/portal/package.json
- scripts/build-public-apps-matrix.js
- scripts/test/public-apps.test.js
- apps/announcement-bar/package.json
- apps/admin-toolbar/package.json
- scripts/release-apps.js
- scripts/test/build-public-apps-matrix.test.js
1 participant
ref https://linear.app/ghost/issue/PLA-235/consolidate-ci-scripts-into-monorepo-package