-
Notifications
You must be signed in to change notification settings - Fork 110
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
Migrate docs
build system from pnpm to bun
#1268
Migrate docs
build system from pnpm to bun
#1268
Conversation
822e79c
to
2dbf9f9
Compare
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.
Reviewable status: 0 of 2 LGTMs obtained, and 0 of 25 files reviewed, and pending CI: Cargo Dev / macos-13 (waiting on @aaronmondal and @blakehatch)
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.
Fantastic change! This finally gives us full control over the entire website deployment process. Also it's way nicer to work with bun and the deno deployment is even faster than cloudflare 🚀
Reviewed 17 of 24 files at r1, 8 of 8 files at r2, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained, and all files reviewed, and pending CI: Cargo Dev / macos-13, and 17 discussions need to be resolved (waiting on @aaronmondal and @blakehatch)
-- commits
line 2 at r2:
Add "Fixes #1082" to the commit message since it fixes that issue.
flake.nix
line 429 at r2 (raw file):
bazel ## Infrastructure Stack
flake.nix
line 447 at r2 (raw file):
pkgs.kustomize ## Web Stack
flake.nix
line 451 at r2 (raw file):
pkgs.deno pkgs.lychee pkgs.nodejs_22 # Only for Pagefind.
Try whether we can get away with pkgs.nodejs-slim_22
which has a smaller closure size.
.github/workflows/native-docs.yaml
line 14 at r2 (raw file):
deploy: name: Docs Deployment runs-on: ubuntu-22.04
Use Ubuntu 24.04 since it might be slightly faster.
.github/workflows/native-docs.yaml
line 29 at r2 (raw file):
run: | nix develop --impure --command bash -c " deno install -Arf jsr:@deno/deployctl &&
Pin the @deno/deployctl
to a hashlocked version.
.github/workflows/native-docs.yaml
line 34 at r2 (raw file):
" - name: Cache Nix derivations
This step should come before the deploy step.
docs/.gitignore
line 34 at r2 (raw file):
stats.html # Deno Deploy File
Suggestion:
file
docs/biome.json
line 10 at r2 (raw file):
"enabled": true, "rules": { "recommended": true,
Could we put this back to "all" and disable only the warnings that really don't make sense (like the naming convention warning because of the generated md_to_mdx and metaphase types)? I'd like this to be as strict as possible.
docs/biome.json
line 17 at r2 (raw file):
} }, "ignore": [".astro", "dist", "node_modules", ".wrangler"]
I think it's fine to remove the .wrangler
here. It does break previous users once, but they should remove the old .wrangler
directory anyways.
docs/package.json
line 17 at r2 (raw file):
"fix": "bun lint && bun format && bun sync", "format": "biome format --write .", "lint": "biome check --write --unsafe .",
Linting should only print linter warnings, not fix them. The format
script should instead do the file transformations.
docs/package.json
line 21 at r2 (raw file):
"serve": "deno run --allow-net --allow-read --allow-env ./dist/server/entry.mjs", "deploy": "deployctl deploy --include=./dist --entrypoint=./dist/server/entry.mjs --save-config", "prod": "bun install && bun docs && bun run build && bun run deploy"
I believe this install
needs to use --frozen-lockfile
to prevent bun from autoupdating the bun.lockb
.
docs/package.json
line 48 at r2 (raw file):
}, "overrides": { "playwright-core": "1.44.0"
Is 1.44.0
correct or do we have to use 1.44.1
like previrously?
docs/README.md
line 71 at r2 (raw file):
## 🐛 Known issues - `bun metaphase` isn't working with the nix version of Bazel,
This is only true on MacOS. Linux doesn't require this.
tools/nixpkgs_bun.diff
line 29 at r2 (raw file):
url = "https://github.com/oven-sh/bun/releases/download/bun-v${version}/bun-darwin-x64-baseline.zip"; - hash = "sha256-5PLk8q3di5TW8HUfo7P3xrPWLhleAiSv9jp2XeL47Kk="; + hash = "sha256-nf75MyU6ye7xHwdxJpaAGcg4MbJ7v2OUgEbepaUE5kg=";
This seems to be the wrong hash: https://github.com/TraceMachina/nativelink/actions/runs/10488998422/job/29052651314?pr=1268#step:6:286
docs/src/styles/custom.css
line 19 at r2 (raw file):
html { scroll-behavior: smooth;
@blakehatch Do we want this globally?
docs/src/utils/metaphase_aot.ts
line 1 at r2 (raw file):
import { join } from "node:path";
Hmm are we sure that biome properly captures this file? This should trigger a node module warning that has to be explicitly ignored.
@blakehatch I believe we need to adjust the github settings so that deno deploy can deploy from this repo. We'll also need to run some tests to ensure that this works properly on PRs. @SchahinRohani We probably need to adjust the workflow with a conditional so that it only builds and lints the docs on PRs but runs the entire deploy step on pushes to main. We can do this by splitting the deno invocations in two steps and using an nativelink/.github/workflows/image.yaml Line 55 in ae0c82c
|
fa34b4f
to
7aa81c0
Compare
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.
Reviewable status: 0 of 2 LGTMs obtained, and 15 of 26 files reviewed, and pending CI: Cargo Dev / macos-13, Docs Deployment, Installation / macos-13, Installation / macos-14, Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (22.04), and 17 discussions need to be resolved (waiting on @aaronmondal and @blakehatch)
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Add "Fixes #1082" to the commit message since it fixes that issue.
done.
flake.nix
line 429 at r2 (raw file):
bazel ## Infrastructure Stack
done.
flake.nix
line 447 at r2 (raw file):
pkgs.kustomize ## Web Stack
done.
flake.nix
line 451 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Try whether we can get away with
pkgs.nodejs-slim_22
which has a smaller closure size.
Looks like we don't even need nodejs for now. It builds on my test/workflow without having nodejs.
Since the Nix Installation on MacOS is failing on #1270 i dont think its a problem with the flake dependencies.
.github/workflows/native-docs.yaml
line 14 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Use Ubuntu 24.04 since it might be slightly faster.
ok.
.github/workflows/native-docs.yaml
line 29 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Pin the
@deno/deployctl
to a hashlocked version.
ok.
.github/workflows/native-docs.yaml
line 34 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
This step should come before the deploy step.
ok.
docs/.gitignore
line 34 at r2 (raw file):
stats.html # Deno Deploy File
ok.
docs/biome.json
line 10 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Could we put this back to "all" and disable only the warnings that really don't make sense (like the naming convention warning because of the generated md_to_mdx and metaphase types)? I'd like this to be as strict as possible.
ok.
docs/biome.json
line 17 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
I think it's fine to remove the
.wrangler
here. It does break previous users once, but they should remove the old.wrangler
directory anyways.
ok.
docs/package.json
line 17 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Linting should only print linter warnings, not fix them. The
format
script should instead do the file transformations.
I have also added a check.lint for only printing linter warnings. It is convenient to work with the bun lint command.
docs/package.json
line 21 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
I believe this
install
needs to use--frozen-lockfile
to prevent bun from autoupdating thebun.lockb
.
The bun.lockb should be frozen now, so the bun install should not make any difference to the lockfile.
docs/package.json
line 48 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Is
1.44.0
correct or do we have to use1.44.1
like previrously?
I used the version thats pinned in the nixpkgs_playwright.diff.
docs/README.md
line 71 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
This is only true on MacOS. Linux doesn't require this.
Rewrited.
tools/nixpkgs_bun.diff
line 29 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
This seems to be the wrong hash: https://github.com/TraceMachina/nativelink/actions/runs/10488998422/job/29052651314?pr=1268#step:6:286
fixed.
docs/src/styles/custom.css
line 19 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
@blakehatch Do we want this globally?
Just have a look on the desktop version on https://nativelink-docs.deno.dev and use the right sidebar. You will see the effect and i would recommend it.
docs/src/utils/metaphase_aot.ts
line 1 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Hmm are we sure that biome properly captures this file? This should trigger a node module warning that has to be explicitly ignored.
fixed
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.
@aaronmondal I have implemented a test deployment on PR and a Production Deployment on push on main.
I believe that's a good strategy to preview test deployments before merging them into main.
@blakehatch To enable automatic authorization for deployments, please log in to https://dash.deno.com/login once. After that, I can invite you to the Nativelink organization, allowing you to manage the deployment of the Nativelink GitHub repository.
Reviewable status: 0 of 2 LGTMs obtained, and 15 of 26 files reviewed, and pending CI: Docs Deployment, Installation / macos-13, Installation / macos-14, Remote / large-ubuntu-22.04, and 17 discussions need to be resolved (waiting on @aaronmondal and @blakehatch)
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.
Reviewable status: 0 of 2 LGTMs obtained, and 15 of 26 files reviewed, and pending CI: Docs Deployment, Installation / macos-13, Installation / macos-14, Remote / large-ubuntu-22.04, and 17 discussions need to be resolved (waiting on @aaronmondal and @blakehatch)
tools/nixpkgs_bun.diff
line 29 at r2 (raw file):
Previously, SchahinRohani (Schahin) wrote…
fixed.
The sha256 of the bun package is the right version, so there was no fix fo this.
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.
Reviewed 11 of 11 files at r3, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained, and all files reviewed, and pending CI: Docs Deployment, Installation / macos-14, and 12 discussions need to be resolved (waiting on @blakehatch)
Previously, SchahinRohani (Schahin) wrote…
done.
Hmm maybe bugged in reviewable? It's ok I can add this when merging.
.github/workflows/native-docs.yaml
line 29 at r2 (raw file):
Previously, SchahinRohani (Schahin) wrote…
ok.
Is there a way to pin this to a hash instead of a version? Versions could be overridden which is a supply-chain security risk.
docs/README.md
line 71 at r3 (raw file):
## 🐛 Known issues - `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel,
Suggestion:
doesn't work
docs/README.md
line 71 at r3 (raw file):
## 🐛 Known issues - `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel,
Suggestion:
.
docs/README.md
line 71 at r3 (raw file):
## 🐛 Known issues - `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel,
Suggestion:
MacOS
docs/README.md
line 72 at r3 (raw file):
- `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel, The solution to this: Install Bun and Bazel on your host and build the docs outside the flake.
Suggestion:
As a workaround i
docs/README.md
line 73 at r3 (raw file):
- `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel, The solution to this: Install Bun and Bazel on your host and build the docs outside the flake. - `bun dev` isn't reloading the changes in the starlight.conf.ts
Suggestion:
doesn't reload
tools/nixpkgs_bun.diff
line 29 at r2 (raw file):
Previously, SchahinRohani (Schahin) wrote…
The sha256 of the bun package is the right version, so there was no fix fo this.
I think there is a duplicate ==
at the end now.
docs/src/utils/md_to_mdx.ts
line 169 at r1 (raw file):
} export function preProcessMarkdown(markdown: string): string {
No need to rename this.
docs/src/utils/md_to_mdx.ts
line 230 at r3 (raw file):
} function isSpecialLine(line: string): boolean {
This should probably be called isAside
: https://starlight.astro.build/guides/components/#asides
Code quote:
SpecialLine
a7afde8
to
4e847c7
Compare
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.
Reviewable status: 0 of 2 LGTMs obtained, and 21 of 26 files reviewed, and pending CI: Analyze (javascript-typescript), Docs Deployment, Publish image, Publish nativelink-worker-init, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, and 12 discussions need to be resolved (waiting on @aaronmondal and @blakehatch)
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Hmm maybe bugged in reviewable? It's ok I can add this when merging.
My bad! Now it should be in the commit message 😄
docs/README.md
line 71 at r3 (raw file):
## 🐛 Known issues - `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel,
Done.
docs/README.md
line 71 at r3 (raw file):
## 🐛 Known issues - `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel,
Done.
docs/README.md
line 71 at r3 (raw file):
## 🐛 Known issues - `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel,
Done.
docs/README.md
line 72 at r3 (raw file):
- `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel, The solution to this: Install Bun and Bazel on your host and build the docs outside the flake.
Done.
docs/README.md
line 73 at r3 (raw file):
- `bun run docs.build` isn't working on `MacOS` with the nix version of Bazel, The solution to this: Install Bun and Bazel on your host and build the docs outside the flake. - `bun dev` isn't reloading the changes in the starlight.conf.ts
Done.
tools/nixpkgs_bun.diff
line 29 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
I think there is a duplicate
==
at the end now.
I think the sha256sum is valid for the bun package.
.github/workflows/native-docs.yaml
line 29 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Is there a way to pin this to a hash instead of a version? Versions could be overridden which is a supply-chain security risk.
I moved it from the workflow in to the package.json.
docs/src/utils/md_to_mdx.ts
line 169 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
No need to rename this.
Ok.
docs/src/utils/md_to_mdx.ts
line 230 at r3 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
This should probably be called
isAside
: https://starlight.astro.build/guides/components/#asides
This is getting more into the actual docs generation, which should not be a part of this PR. This is only a quick fix for the successful integration of Biome without any warnings and errors.
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.
@blakehatch and I need to set up the repo for this before we can merge, but I think this is good now. Great job!
Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed, and pending CI: Installation / macos-13, docker-compose-compiles-nativelink (22.04), integration-tests (22.04) (waiting on @blakehatch)
4e847c7
to
c2f7222
Compare
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.
Everything functional for me and significantly faster cuz of Bun.
Reviewable status: complete! 2 of 2 LGTMs obtained, and all files reviewed
docs/src/styles/custom.css
line 19 at r2 (raw file):
Previously, SchahinRohani (Schahin) wrote…
Just have a look on the desktop version on https://nativelink-docs.deno.dev and use the right sidebar. You will see the effect and i would recommend it.
For docs I would like it globally.
c2f7222
to
e6a428e
Compare
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'll merge as soon as we have the deno deploy app set up.
Reviewed 7 of 7 files at r5, all commit messages.
Reviewable status: complete! 2 of 2 LGTMs obtained, and all files reviewed
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.
Reviewable status: complete! 2 of 2 LGTMs obtained, and all files reviewed
Description
Fixes #1082
Preview Deploy NativeLink Docs Github Action Workflow
Preview the deployed NativeLink Docs
Type of change
Please delete options that aren't relevant.
Checklist
bazel test //...
passes locallygit amend
see some docsThis change is