search

tidepool-org / cp2102

Silicon Labs CP2102 user-space USB driver for Node.js
BSD 2-Clause "Simplified" License
16 stars 4 forks source link

bumping usb to v1.6.3 #8

Closed simonarnell closed 3 years ago

simonarnell commented 3 years ago

Apologies @gniezen that was only supposed to be a version bump, I forgot subsequent commits on the branch would appear in the pr.

Regarding your comment on my extraneous commit, I thought by checking for this.setup as undefined / null, it would then default to the setup object that is equivalent to the project’s original. Or am I misunderstanding and you’d prefer me not to alter the constructor’s signature, and instead pass as a setup property in the opts argument?

gniezen commented 3 years ago

I'm happy for you to alter the constructor's signature to allow for alternative setups, I just want the control transfers in the default setup to stay the same as the original setup, as not to break existing implementations using this library. The second and third control transfers are different at the moment.

simonarnell commented 3 years ago

@gniezen Merde, I must have copy and pasted the wrong setup - thanks for catching! I've now reverted my subsequent edits from this PR and will create a second PR covering off the new functionality, such that this PR only covers the usb v1.6.3 bump, as titled.

gniezen commented 3 years ago

@simonarnell Your changes are looking good, but the PR was created against your own repo, so I can't merge it into this one. I'm going to merge this one, and then you can create a new PR against the main branch.

gniezen commented 3 years ago

Oh, I almost forgot. Would you mind agreeing to the Tidepool Contributor License Agreement so that I can accept your changes? See https://developer.tidepool.org/contributors/ for details - you can just reply with the following on this PR:

I agree to the terms of Tidepool Project’s Volunteer/Contributor License Agreement v1.1 as it exists at http://tidepool-org.github.io/files/TidepoolVCLA-1.1.pdf on <TODAY’S DATE>.

gniezen commented 3 years ago

@simonarnell If you submit another PR for the setup changes, I'll get that merged as well and push out a new release on npm.