[PM-12967] Create bitwarden-wasm-internal #1089
Conversation
|
New Issues
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1089 +/- ##
==========================================
- Coverage 58.44% 58.32% -0.13%
==========================================
Files 192 193 +1
Lines 13528 13557 +29
==========================================
Hits 7907 7907
- Misses 5621 5650 +29 ☔ View full report in Codecov by Sentry. |
Hinton
changed the title
| - name: Publish NPM | ||
| if: ${{ github.ref == 'refs/heads/main' }} | ||
| run: npm publish --access public | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| working-directory: languages/js/sdk-internal |
There was a problem hiding this comment.
We should split the build and publish workflows. If it's needed to publish it on every push to main, I suggest triggering the publishing workflow here. An example of that can be found in the build-swift.yml workflow: https://github.com/bitwarden/sdk/blob/90f8a5461e5e7ed07b1f5a13fc2fb793994a6fb5/.github/workflows/build-swift.yml#L101-L115
BTW is it released to npmjs.com or to github packages?
There was a problem hiding this comment.
Currently gh packages, but we will probably change it to npmjs.
…m-internal # Conflicts: # Cargo.toml # crates/bitwarden-core/Cargo.toml # crates/bitwarden/Cargo.toml
| js-sys = "0.3.68" | ||
| log = "0.4.20" | ||
| serde_json = ">=1.0.96, <2.0" | ||
| wasm-bindgen = { version = "0.2.91", features = ["serde-serialize"] } |
There was a problem hiding this comment.
question: should this use the workspace version?
wasm-bindgen = { workspace = true }
I haven't seen this syntax before, but I saw that you did this in crates/bitwarden-core/Cargo.toml
There was a problem hiding this comment.
No, we use an explicit version here while the workspace uses ranges.
There was a problem hiding this comment.
Oh ok, is that because this version is used for building and in that case wasm-bindgen and wasm-bindgen-cli versions need to be exact, while for the other stuff we can be more relaxed with the version? Do you think that different versions can cause issues? Or should it be fine since we're using the same minor versions?
| } | ||
| } | ||
|
|
||
| // Rc<...> is to avoid needing to take ownership of the Client during our async run_command |
There was a problem hiding this comment.
question/issue?: We don't have a run_command anymore?
There was a problem hiding this comment.
Nop, but copy pasta until we figure out if we need RC here or not 🤷
|
@Hinton the PR has workflow linting issues |
|
@coroiu those aren't from this PR. |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-12967
📔 Objective
PM SDK. Based on the research spike in #1077
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes