-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
fix(OpenNext): build blog data #8228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Aviv Keller <[email protected]>
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Signed-off-by: Aviv Keller <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8228 +/- ##
=======================================
Coverage 76.44% 76.44%
=======================================
Files 115 115
Lines 9643 9643
Branches 316 316
=======================================
Hits 7372 7372
Misses 2270 2270
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Aviv Keller <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a pre-build step to generate blog data before deploying the OpenNext Cloudflare worker so the site has up-to-date content at deploy time.
- Introduces a "Build blog data" step in the site workflow.
- Runs the blog data build in the apps/site directory prior to the OpenNext worker build.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Can we dispatch the workflow against this branch to confirm this fix works? |
I am not sure why this is necessary, wasn't the blog building done in a postinstall script? |
Ah yeah there are nodejs.org/apps/site/package.json Line 5 in bd92fd2
|
|
I did when I opened the PR, apologies for not linking the run, https://github.com/nodejs/nodejs.org/actions/runs/18532159595 |
Even if it did, those lifecycle scripts wouldn't apply to the worker variants, right? |
ah ok, thanks for the extra context 🙂 |
oh yeah you're right 😕 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🙂
@nodejs/web-infra feel free to merge once approved, I'm requesting fast track |
We need to build the blog data before deploying