-
Notifications
You must be signed in to change notification settings - Fork 27
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
Upgrade to Node v18, fix deploy-preview CI job #236
Conversation
It tried to extract the deploy-preview CI script fix to a separate PR, but I can't install the latest version of octokit without bumping Node. So I'd like to do it in this PR.
|
@@ -15,7 +15,7 @@ | |||
"typescript": "~4.6.2" | |||
}, | |||
"engines": { | |||
"node": ">=12" | |||
"node": ">=18" |
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.
I think Node 18 is ok to declare as a minimum engine, since Node 16 maintenance LTS is about to end at the end of this month. See Node.js Releases. @blueprintjs/node-build-scripts
already declares a minimum Node version of 18.13.
artifacts=$(curl -X GET "https://circleci.com/api/v2/project/github/palantir/documentalist/$CIRCLE_BUILD_NUM/artifacts" -H "Accept: application/json" -u "$CIRCLE_AUTH_TOKEN:") | ||
echo $artifacts > ./scripts/artifacts.json | ||
node ./scripts/circle-build-preview.js | ||
- run: ./scripts/submit-preview-comment.sh |
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.
is this PR supposed to hav ea comment on it?
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.
good Q, the preview comment is not working since we don't have the API token being injected right now but we do have docs built and saved as an artifact, which is most of the important part: https://app.circleci.com/pipelines/github/palantir/documentalist/389/workflows/8aba4d88-d87e-4dd5-a45f-ffdda3fad7a5/jobs/2679/artifacts
return `<a href="${artifactInfo?.url}">${pkg}</a>`; | ||
} | ||
|
||
if (process.env.GITHUB_API_TOKEN) { |
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.
looks like there are some old tokens in the CI env variables right now, I'm going to try those out. it those don't work though, I will have to generate & inject a new personal token just like palantir/blueprint#5827 and the preview comments will come from my GitHub account
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.
adjust curl cmdBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
fi | ||
|
||
SCRIPTS_DIR=$(dirname "$(readlink -f "$0")") | ||
artifacts=$(curl --request GET --url "https://circleci.com/api/v2/project/gh/$CIRCLE_PROJECT_USERNAME/$CIRCLE_PROJECT_REPONAME/$CIRCLE_BUILD_NUM/artifacts" --header "authorization: $CIRCLE_API_TOKEN") |
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.
adjusted this based on docs: https://circleci.com/docs/api/v2/index.html#operation/getJobArtifacts
I'm thinking of migrating this to be completely implemented in JS and exporting it for re-use from @blueprintjs/node-build-scripts
preview comments work now! but they have the wrong package links 😅 |
Build preview link for commit "update comment body": documentation This is an automated comment from the deploy-preview CircleCI job. |
Pre-commit for #156
This will allow us to upgrade to TypeScript v5.x which has
lib.d.ts
changes incompatible with Node 16