Remove vault folder from wasm-internal #294
Conversation
| pub fn generate_totp_wasm( | ||
| &self, | ||
| key: String, | ||
| time_ms: Option<f64>, |
There was a problem hiding this comment.
If we're having a separate WASM function to deal with the parameters, we could use js_sys::Date or js_sys::Number here and then convert, that'll probably be a breaking change for the current SDK API though, so maybe later.
Hinton
requested review from
dani-garcia and
coroiu
and removed request for
addisonbeck
|
Great job, no security vulnerabilities found in this Pull Request |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #294 +/- ##
==========================================
+ Coverage 70.20% 70.28% +0.07%
==========================================
Files 220 217 -3
Lines 17009 16990 -19
==========================================
Hits 11942 11942
+ Misses 5067 5048 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| pub fn generate_totp_wasm( | ||
| &self, | ||
| key: String, | ||
| time_ms: Option<f64>, |
There was a problem hiding this comment.
If we're having a separate WASM function to deal with the parameters, we could use js_sys::Date or js_sys::Number here and then convert, that'll probably be a breaking change for the current SDK API though, so maybe later.
## 🎟️ Tracking <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective <!-- Describe what the purpose of this PR is, for example what bug you're fixing or new feature you're adding. --> Per request in #294 we want to avoid having multiple impl of VaultClient. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+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 confirmed issue 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
|
🎟️ Tracking
📔 Objective
Removes the vault folder from wasm-internal and relies on the upstream implementations instead. Note this adds
wasm_bindgento everything that should be exposed which is a bit clunky.⏰ 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