bloomberg / blpapi-node

Bloomberg Open API module for node.js
http://www.bloomberglabs.com/api/about/
Other
246 stars 50 forks source link

Moved wrapper from blpapi-http project into blpapi-node #50

Closed thmiceli closed 9 years ago

thmiceli commented 9 years ago

The wrapper from blpapi-http has been moved into this project. This allows the removal of one layer of function calls forwarding and simplifies the blpapi-node interface. Examples have been updated. To review the main code changes it may be useful to compare the blpapi-wrapper.ts file (from blpapi-http) to the new blpapi.ts file in this project.

ericvw commented 9 years ago

@apaprocki - would appreciate your eyes on this one before we decided to merge to master.

thmiceli commented 9 years ago

Looks like CI is not pulling the right node version (0.12), thus the build errors. From the build logs:

nvm install 0.12 ... $ node --version v0.10.36

thmiceli commented 9 years ago

I pushed an update for the nodejs type definition file.

ericlu88 commented 9 years ago

Please rebase against the latest changes of the binding layer.

Note: The request/subscribe call to the native code can't have undefined for the optional parameters(identity, label, etc).

thmiceli commented 9 years ago

Update with new 'generateToken' and 'authorize' functions that use the newly exposed authentication/authorization functions from the binding layer. I also added overloads for 'request' and 'subscribe' functions and optional arguments are now forwarded properly.

thmiceli commented 9 years ago

Updated PR which I believe addresses the comments from @ericlu88.

ericvw commented 9 years ago

Is this in a state to get another pass for review?

thmiceli commented 9 years ago

Yes I just pushed the latest change suggested by @ericlu88 (proper import of the blpapijs module).

thmiceli commented 9 years ago

The typescript compilation and lint are now run through node-gyp and will run in the 'prepublish' phase. That works when running locally 'npm install'. I tested 'publishing' through 'npm pack', that works too but I am not sure that is totally equivalent.

ericlu88 commented 9 years ago

@thmiceli Can you switch to use tsd to manage the typing files? This will make it easier for future development.

ericvw commented 9 years ago

+1 for using tsd to manage external typing files.

thmiceli commented 9 years ago

I addressed the latest comments and updated the PR.

ericvw commented 9 years ago

lgtm