Skip to content
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

getrandom: Don't set "wasm-bindgen" #56

Merged
merged 1 commit into from
Jun 22, 2021
Merged

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented Jun 22, 2021

The only stable features of getrandom are:

  • "std"
  • "rdrand"
  • "js"
  • "custom"

Setting "wasm-bindgen" isn't guaranteed to work going forward (and does nothing currently).

Just to note, your crate is also setting the "js" feature. This is basically saying, "every wasm32-unknown-unknown user of this crate is either on Node.js or Client-side Web, and has access to JavaScript". This is a very reasonable restriction for your use case (as this project seems to have explicit JavaScript bindings), but we always want dependent library crates to be aware of this.

Signed-off-by: Joe Richey [email protected]

The only supported features of `getrandom` are:
  - "rdrand"
  - "js"
  - "custom"

Setting `"wasm-bindgen"` isn't garunteed to work going forward (and does
nothing currently).

Just to note, your crate is also setting the "js" feature. This is
basically saying, "every `wasm32-unknown-unknown` user of this crate is
either on Node.js or Client-side Web, and has access to JavaScript".
This is a reasonable restriction for your use case, but we always want
dependant library crates to be aware of this.

Signed-off-by: Joe Richey <[email protected]>
@josephlr
Copy link
Contributor Author

CC: #13 rust-random/getrandom#219

@codecov-commenter
Copy link

Codecov Report

Merging #56 (8c60e1a) into master (4007939) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #56   +/-   ##
=======================================
  Coverage   83.77%   83.77%           
=======================================
  Files          11       11           
  Lines         912      912           
=======================================
  Hits          764      764           
  Misses        148      148           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4007939...8c60e1a. Read the comment docs.

@fjarri
Copy link
Contributor

fjarri commented Jun 22, 2021

Thanks for the PR, I am not sure how wasm-bindgen ended up there — might have been a remainder from getrandom-0.1.

@fjarri fjarri merged commit e2267b5 into nucypher:master Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants