-
Notifications
You must be signed in to change notification settings - Fork 30
switch skipruntime-ts to pnpm #974
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
base: main
Are you sure you want to change the base?
Conversation
hubyrod
commented
Jul 20, 2025
- fix Makefile
- switch skipruntime-ts to pnpm
e17821b to
a8a27bc
Compare
| submodules: true | ||
| - run: | ||
| name: Install pnpm | ||
| command: npm install -g pnpm |
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.
Opinions vary but FWIW I tend to prefer using the long form of options when written out in scripts, etc. to make it easier to read for people who might not have the short forms memorized.
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.
Absolutely. I'll make command line options explicit.
| .PHONY: npm | ||
| npm: $(SKDB_WASM) build/package/skdb build/package/package.json | ||
| cd build/package && npm install | ||
| .PHONY: pnpm | ||
| pnpm: | ||
| pnpm install && pnpm build | ||
|
|
||
| build/package/package.json: | ||
| @echo "{" > build/package/package.json | ||
| @echo " \"dependencies\": {" >> build/package/package.json | ||
| @echo " \"skdb\": \"file:skdb\"" >> build/package/package.json | ||
| @echo " }" >> build/package/package.json | ||
| @echo "}" >> build/package/package.json |
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.
Something you might be hitting that isn't obvious is that here the npm stuff is referring to the support for using skdb from js using npm packages.
| npm install && npm run build | ||
| cd www && rm -rf docs/api && npm install && npx docusaurus generate-typedoc | ||
| pnpm install && pnpm build | ||
| cd www && rm -rf docs/api && pnpm install && npx docusaurus generate-typedoc |
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.
The various npx invocations should be changed to something pnpm friendly, right? Maybe pnpm exec?
|
|
||
| cd "$REPO" | ||
| DIRS=$( jq --raw-output ".workspaces[]" package.json ) | ||
|
|
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.
Note that the line above reads the workspaces field of the package.json file that isn't here anymore.
| # For pnpm, use filter patterns instead of individual -w flags | ||
| PNPM_SKIPRUNTIME_FILTER=--filter="skipruntime-ts/core" --filter="skipruntime-ts/helpers" --filter="skipruntime-ts/wasm" --filter="skipruntime-ts/addon" --filter="skipruntime-ts/adapters/kafka" --filter="skipruntime-ts/adapters/postgres" --filter="skipruntime-ts/tests" --filter="skipruntime-ts/server" --filter="skipruntime-ts/examples" --filter="skipruntime-ts/metapackage" --filter="skiplang/prelude/ts/binding" --filter="skiplang/prelude/ts/wasm" --filter="skiplang/prelude/ts/worker" --filter="skiplang/skdate/ts" --filter="skiplang/skjson/ts/binding" --filter="skiplang/skjson/ts/wasm" --filter="eslint-config" --filter="tsconfig" |
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.
The previous version has the desirable quality that it only explicitly mentions the things we want to exclude, and so will automatically include anything new. Is there no way to preserve that characteristic?
| .PHONY: build-examples | ||
| build-examples: build | ||
| ../bin/cd_sh examples "npm run build -w skipruntime-examples" | ||
| @echo "Skipping examples build due to dependency issues" |
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 expected to be temporary, or is there some bigger issue?