-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Packaging #2
Conversation
@@ -8,8 +8,16 @@ use mina_signer::{NetworkId, PubKey, Schnorr, Signer}; | |||
use once_cell::sync::OnceCell; | |||
use std::io::Write; | |||
|
|||
#[wasm_bindgen(typescript_custom_section)] |
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.
Maybe also convert examples to use typescript and run linter to verify these type definitions
{ | ||
"name": "mina-singer-wasm", | ||
"description": "Lightweight implementation of mina-signer build into wasm package for browser and nodejs.", | ||
"version": "0.1.0", |
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.
How about matching the version of mina-signer
, which is v1.1.x in the current implementation
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.
Hmm, I would rather if we start fresh with 1.0.0. but put in readme table with what our versions are compatible with 1.1.x. Otherwise, if we would need to release some fix we would break parity.
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.
If we keep the api compatibility, I don't see why we would release a version that bumps the minor, isn't the patch version sufficient for potential fixes by still complying with semver?
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.
I'm voting to be independent versioning
The version used here is from cargo.toml
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.
If we keep the api compatibility, I don't see why we would release a version that bumps the minor, isn't the patch version sufficient for potential fixes by still complying with semver?
It is but it's also confusing as we would be for example 1.1.16 while they are on 1.1.0 and devs don't draw the connection. It also prevents us from adding some valuable features like different ways of initialization or breaking changes like converting to es module only or improving types.
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.
ci parts moved to #3
npm
release