Skip to content
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

refactor: updated release script and updated shiki #5997

Merged
merged 9 commits into from
Oct 15, 2023

Conversation

ovflowd
Copy link
Member

@ovflowd ovflowd commented Oct 13, 2023

Description

This PR cleans up and refactors the Node.js Release Blog Post generation script.

  • Modernizes the code a little bit
  • Cleanses the code
  • Updates the code by removing a few outdated things (regex replacements, version checks, and other things)
  • Fixes occasional "Coming Soon" due to using "direct.nodejs.org"
  • More obvious promisification of pieces of the code
  • Already transforms the * lists into - lists (sadly prettier does not allow this to be changed)
  • Adds ShellScript and ShellSession Shiki Languages

Validation

Generating the Blog Posts should still work (for latest version, specific version, and forced generation)

@ovflowd ovflowd requested a review from a team as a code owner October 13, 2023 23:31
@vercel
Copy link

vercel bot commented Oct 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 14, 2023 9:02am

@ovflowd
Copy link
Member Author

ovflowd commented Oct 13, 2023

cc @tniessen @nodejs/web-infra

Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

RSLGTM :)

@github-actions
Copy link

github-actions bot commented Oct 13, 2023

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 95%
94.68% (267/282) 78.12% (50/64) 92.98% (53/57)

Unit Test Report

Tests Skipped Failures Errors Time
27 0 💤 0 ❌ 0 🔥 5.646s ⏱️

Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

read through it - also ran the current release post again

@richardlau
Copy link
Member

The preview link is showing a 500 error?

@richardlau
Copy link
Member

* Fixes occasional "Coming Soon" due to using "direct.nodejs.org"

History is that we switched to using direct because of "Coming Soon"'s occurring: #4136
This will be very hard to validate/test outside of a release.

@bmuenzenmeyer
Copy link
Collaborator

The preview link is showing a 500 error?

I'm guessing that's a hiccup - local and CI deploy and serve scripts run fine and this changeset doesn't appear to have anything related to next deployment. 🤞

@ovflowd or someone else with Vercel access can look into it (I still do not)

@ovflowd ovflowd changed the title meta: updated shiki package version refactor: updated release script and updated shiki Oct 14, 2023
@ovflowd
Copy link
Member Author

ovflowd commented Oct 14, 2023

* Fixes occasional "Coming Soon" due to using "direct.nodejs.org"

History is that we switched to using direct because of "Coming Soon"'s occurring: #4136 This will be very hard to validate/test outside of a release.

Right. But that's the source of flakiness. The thing is that direct.nodejs.org is less reliable, as it is a non-cached origin. Not to mention once we move to R2, direct.nodejs.org is not an endpoint we want to hit.

@ovflowd
Copy link
Member Author

ovflowd commented Oct 14, 2023

The preview link is showing a 500 error?

I'm guessing that's a hiccup - local and CI deploy and serve scripts run fine and this changeset doesn't appear to have anything related to next deployment. 🤞

@ovflowd or someone else with Vercel access can look into it (I still do not)

Yea, I fixed the issue. It was shiki's embeddedLangs. Apparently it completely ignores the language grammar I am providing and tries to use its "built-in" resolution.

Removing embeddedLangs surprisingly doesn't break the highlighting, and apparently it is only used to pre-load other languages before the one you're currently loading. But since our shiki.config.mjs is already ordered, all fine. I swear, I hate when popular libraries depend on filesystem or other internals and give little to none control to end-users (I had to dig deep in order to make this shiki.config.mjs to work to begin with.

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM

@ovflowd ovflowd added this pull request to the merge queue Oct 15, 2023
Merged via the queue into main with commit dafcdaa Oct 15, 2023
14 checks passed
@ovflowd ovflowd deleted the feat/release-post-and-shiki branch October 15, 2023 09:56
@richardlau
Copy link
Member

* Fixes occasional "Coming Soon" due to using "direct.nodejs.org"

History is that we switched to using direct because of "Coming Soon"'s occurring: #4136 This will be very hard to validate/test outside of a release.

Right. But that's the source of flakiness. The thing is that direct.nodejs.org is less reliable, as it is a non-cached origin. Not to mention once we move to R2, direct.nodejs.org is not an endpoint we want to hit.

Yes, being non-cached was the point behind the original switch. Maybe it's not an issue now with the tweaks that were made to avoid caching 404's.

@ovflowd
Copy link
Member Author

ovflowd commented Oct 16, 2023

* Fixes occasional "Coming Soon" due to using "direct.nodejs.org"

History is that we switched to using direct because of "Coming Soon"'s occurring: #4136 This will be very hard to validate/test outside of a release.

Right. But that's the source of flakiness. The thing is that direct.nodejs.org is less reliable, as it is a non-cached origin. Not to mention once we move to R2, direct.nodejs.org is not an endpoint we want to hit.

Yes, being non-cached was the point behind the original switch. Maybe it's not an issue now with the tweaks that were made to avoid caching 404's.

Right. I'll keep an eye on that once a next release is done!

@targos
Copy link
Member

targos commented Oct 24, 2023

I'm releasing v21.1.0 and hit two "coming soon" links and no shasums in the generated changelog.

@targos
Copy link
Member

targos commented Oct 24, 2023

Rerunning a couple times fixed it, but it seems that the warning that used to be printed is no longer here so this is prone to errors if the generated file is not verified.

@ovflowd
Copy link
Member Author

ovflowd commented Oct 24, 2023

Rerunning a couple times fixed it, but it seems that the warning that used to be printed is no longer here so this is prone to errors if the generated file is not verified.

Heyo, 👋 which warnings?

The rerunning is due to Cloudflare giving errors probably a cache purge happened recently.

@targos
Copy link
Member

targos commented Oct 24, 2023

which warnings?

The script used to print warnings to the console when there were lines with "coming soon".

@ovflowd
Copy link
Member Author

ovflowd commented Oct 24, 2023

which warnings?

The script used to print warnings to the console when there were lines with "coming soon".

It should still be doing that if the request failed. But I can double-check. Afaik if the request failed (i.e. it will trigger a coming soon) it should console.warn that it failed. Will give a double check over here.

@targos
Copy link
Member

targos commented Oct 24, 2023

@richardlau also had two "coming soon" links. Did you see a warning?

@richardlau
Copy link
Member

@richardlau also had two "coming soon" links. Did you see a warning?

No, no warnings.

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.

7 participants