node binding generation follow up #834
Conversation
…t/rust-sdks into lukas/node-binding-follow
.github/workflows/node-builds.yml
Outdated
| - name: Build TS parts | ||
| working-directory: livekit-ffi-node-bindings | ||
| run: pnpm build:ts | ||
| run: | |
There was a problem hiding this comment.
should this run before the commit portion?
There was a problem hiding this comment.
Was debating whether or not to move the generate_proto step to before the commit, but felt it was a bit unexpected that only the release workflow would commit the generated proto files.
This currently errors out if the committed protos aren't up to date on main.
I can see three approaches that have their own tradeoffs:
- keep it as is and error out if not up to date
- run on release and commit any changes without double checking that the output is sane
- run
./generate_proto.shon every PR instead and commit (this is what python is doing)
thinking about it again now, I prefer the third option of running it on every PR/build step, does that sound good to you, too?
There was a problem hiding this comment.
I went with the approach that python also does now and commit the proto stubs on every PR
| }, | ||
| "scripts": { | ||
| "build:ts": "tsup --onSuccess \"tsc --declaration --emitDeclarationOnly\" && cp -r src/napi dist/ && cp -r src/napi/* dist/", | ||
| "build": "./generate_proto.sh && pnpm build:ts && napi build --platform --release --dts native.d.ts --js native.cjs --pipe \"prettier -w\" src/napi", |
There was a problem hiding this comment.
we don't need this step anymore? when doing local dev?
There was a problem hiding this comment.
I added this part in the previous PR, but running it like this errors out on windows builds, so I kept it as a separate/manual step in the CI as it was before
…t/rust-sdks into lukas/node-binding-follow
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
follow up for #829