-
Notifications
You must be signed in to change notification settings - Fork 0
CI: JSR publish #7
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
Conversation
Replaces the old embed_so.ts and convert_binary_to_json.ts scripts with a new embed_binary.ts script for embedding binaries as JSON. Updates the GitHub Actions workflow to use the new script, adds dry-run publishing, and cleans up related files. Also updates jsr.json.in export path for consistency.
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.
Pull request overview
This PR refactors the JSR publishing workflow by replacing the TypeScript embedding script with a simpler Bash implementation. The changes streamline the build process and update the package structure to use a cleaner import path (./bin/x84_64 instead of ./x84_64), while removing the sha256 hash from the JSON output. Additionally, a dry-run publish step has been added for non-main branch pushes to validate changes before merging.
Key changes:
- Replaced TypeScript script (
embed_so.ts) with a more concise Bash script (embed_binary.sh) - Updated import path structure from
@<scope>/<package>/x84_64to@<scope>/<package>/bin/x84_64 - Modified workflow to support dry-run publishing on all branch pushes with actual publishing only on main/tags
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| jsr/scripts/embed_so.ts | Deleted the original TypeScript embedding script |
| jsr/scripts/embed_binary.sh | New Bash script that encodes binaries to base64 JSON format |
| jsr/package/jsr.json.in | Updated export path to ./bin/x84_64 and changed license field to "LICENSE" |
| jsr/package/README.md | Updated import path in usage example and removed license section |
| jsr/package/.gitignore | Added jsr.json to ignore list (generated by CMake) |
| jsr/bin/x84_64.json | Removed placeholder JSON file (now generated during build) |
| CMakeLists.txt | Updated paths to reflect new jsr/package directory structure |
| .gitignore | Added global *.so pattern to ignore generated shared libraries |
| .github/workflows/publish-jsr.yml | Updated workflow to use new Bash script and added dry-run publish step for non-main branches |
Comments suppressed due to low confidence (3)
jsr/package/jsr.json.in:7
- Typo in architecture identifier: "x84_64" should be "x86_64". This affects the export path definition for the package, which will break the import path for consumers.
jsr/package/README.md:11 - Typo in architecture identifier: "x84_64" should be "x86_64". This affects the documented import path in the usage example.
jsr/package/jsr.json.in:4 - The license field should specify an SPDX license identifier (e.g., "LGPL-3.0-only"), not a file path. According to JSR documentation, the license field should contain a valid SPDX license identifier. If the license is in a separate file, that file should be included in the publish section (which it is on line 12), but the license field itself should contain the identifier.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Corrects the export path and filename for x86_64 in jsr.json.in. Updates embed_binary.sh to use only the binary's filename in the generated JSON, rather than the full path.
|
@Katze719 everything good to go? |
Co-authored-by: Katze719 <38188106+Katze719@users.noreply.github.com>
Co-authored-by: Katze719 <38188106+Katze719@users.noreply.github.com>
Co-authored-by: Katze719 <38188106+Katze719@users.noreply.github.com>
This PR refactors the whole publishing to
jsr.io.@<scope>/<package>/x86_64to@<scope>/<package>/bin/x86_64to allow for cleaner path structure