Skip to content

[BAC-513] Add public clients [BAC-516]#2

Merged
samweinberg23 merged 3 commits intomasterfrom
sw_public
Oct 21, 2020
Merged

[BAC-513] Add public clients [BAC-516]#2
samweinberg23 merged 3 commits intomasterfrom
sw_public

Conversation

@samweinberg23
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Oct 20, 2020

@samweinberg23 samweinberg23 requested a review from Kenadia October 20, 2020 22:43
Copy link
Contributor

@Kenadia Kenadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(let's use axios instead of request-promise-native)

tsconfig.json Outdated
"compilerOptions": {
"outDir": "build"
"outDir": "build",
"esModuleInterop": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is part of the base config already

}
}

async function sendPublicGetRequest(uri: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be private

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or protected)

// TODO make production
await request({
method: 'GET',
uri: `https://api.stage.dydx.exchange/${uri}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass in host: string, in the constructor


async function sendPublicGetRequest(uri: string): Promise<void> {
// TODO make production
await request({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's return the result even if we're not using it yet -- can use type Promise<{}>

Copy link
Contributor

@Kenadia Kenadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved with some comments

package.json Outdated
"devDependencies": {
"@dydxprotocol/node-service-base-dev": "0.0.10"
"@dydxprotocol/node-service-base-dev": "0.0.10",
"@types/request-promise-native": "^1.0.17"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

getStats(): void {}
getTrades(): void {}
getHistoricalFunding(): void {}
host: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readonly

}

private sendPublicGetRequest(requestPath: string): Promise<{}> {
// TODO make production
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

@samweinberg23 samweinberg23 merged commit 3030ce2 into master Oct 21, 2020
@samweinberg23 samweinberg23 deleted the sw_public branch October 21, 2020 00:12
@Kenadia Kenadia changed the title Add public clients [BAC-516] [BAC-513] Add public clients [BAC-516] Oct 23, 2020
@linear
Copy link

linear bot commented Oct 23, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants