servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.
Mozilla Public License 2.0
293 stars 117 forks source link

Modify TypedArray::create API to accept either a length or an initial slice #336

Closed jdm closed 7 years ago

jdm commented 7 years ago

For cases when we create a new typed array with an initial slice, we always want to use the length of the provided slice. Passing the same length as another argument is redundant; we should use a custom enum argument instead (eg. CreateWith::Length(5) and CreateWith::Slice(foo)).

Code: src/typedarray.rs Tests: cargo test --name typedarray (tests/typedarray.rs)

simon-whitehead commented 7 years ago

This looks like a great first PR. Mind if I give it a shot? I may need some guidance around code/test style but hopefully I won't need too much hand-holding (I can read and write Rust .. I just haven't contributed to Servo yet).

jdm commented 7 years ago

Please do!

simon-whitehead commented 7 years ago

@jdm Are you able to give me some guidance/documentation around versioning? I imagine I'll be required to bump the crate version? How does that affect how this crate version is applied to servo later? I'm just curious given I'll be changing a function signature that may have ripple effects for consumers.

jdm commented 7 years ago

We use a git dependency for rust-mozjs in Servo, so the crate version doesn't actually matter for that case. This crate is also not published to crates.io, so we generally have not been updating the version.

KiChjang commented 7 years ago

Fixed by #337.