Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix get order api request #136

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

dmu1313
Copy link
Contributor

@dmu1313 dmu1313 commented Mar 31, 2024

The client is trying to go to /v4/orders${orderId} and is missing the / that is required between orders and ${orderId}.

@dmu1313
Copy link
Contributor Author

dmu1313 commented Apr 3, 2024

@johnqh @jaredvu @ruixhuang Tagging some random dydx team members for visibility. This is a super simple fix. Can you approve and merge?

@jaredvu
Copy link
Contributor

jaredvu commented Apr 3, 2024

Thanks for your contribution @dmu1313. Very much appreciated 🙏

@jaredvu
Copy link
Contributor

jaredvu commented Apr 3, 2024

Is it possible for you to enable commit signing and also bump the version using ./scripts/bump_version.sh? Commit signing is required on all dYdX OS repos

@dmu1313 dmu1313 force-pushed the fix-get-order-request branch from acc6b3f to 69ec828 Compare April 4, 2024 06:43
@dmu1313
Copy link
Contributor Author

dmu1313 commented Apr 4, 2024

@jaredvu Thanks for the review.

I've done what you asked, although, I just manually changed the version number in package.json rather than using the script you shared. I hope that's ok. The sed command was failing on my machine (apparently sed works slightly differently on linux vs mac according to copilot, so I think that was the issue).

@jaredvu
Copy link
Contributor

jaredvu commented Apr 4, 2024

@dmu1313 Thanks! I think manually changing package.json is ok, you will need to run npm i in order to update the lockfile. Could you bump to 1.1.5 and also run npm i?

The client is trying to go to `/v4/orders${orderId}` and is missing the
`/` that is required between `orders` and `${orderId}`.
@dmu1313 dmu1313 force-pushed the fix-get-order-request branch from 69ec828 to d708a89 Compare April 5, 2024 08:05
@dmu1313
Copy link
Contributor Author

dmu1313 commented Apr 5, 2024

@jaredvu Done.

@jaredvu jaredvu merged commit 305c076 into dydxprotocol:main Apr 5, 2024
@dmu1313 dmu1313 deleted the fix-get-order-request branch April 5, 2024 20:44
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