Closed Boddlnagg closed 7 years ago
The merge commit that you pushed introduced a failure. I force-pushed a rebased version with the fix (let's see if Travis agrees that it's correct 😉 ). And thanks for the review!
@chris-zen Thanks for the review! I will push a commit with the changes that you requested (is it okay if I just put them all in a single commit?). Actually, I am responsible for the 2.0 version of coremidi-sys
, after looking at this library and talking to the developer of coremidi-sys
1.0, who happens to be a friend of mine 😉
I pushed a commit addressing the review comments
Thanks! Do you plan to release a new version soon? Then I will be able to update midir :-)
Done. Released 0.3.0
This adresses the points discussed in #5. Details:
&[u8]
to not require the caller to allocate aVec
PacketBuffer
type.new()
constructor has been removed, because having the buffer always contain at least one package allows some optimizations and I can't see reason for ever needing an emptyPacketBuffer
.new
now has the signature offrom_data
instead.with_data
topush_data
, because I think of it more as similar to theVec
API instead of a builder API.push_data
still returns&mut self
, so it can be chained. If you want me to undo these API changes, I can do that.PacketList::length()
tolen()
to make it consistent with Rust's standard library (all the methods to get the length of something are calledlen()
, see https://static.rust-lang.org/doc/master/std/index.html?search=length)push_data
. This matches the behavior ofMIDIPacketListAdd
, and there are unit tests to ensure this (by comparing the result of usingMIDIPacketListAdd
vs.PacketBuffer
).This PR also includes an upgrade to version 2.0 of coremidi-sys (the first two commits), getting rid of
coremidi_sys_ext
. If you want to have these as a seperate PR, I can do that, too.