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.

Sign up for GitHub

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

Jump to bottom

TRAD-259 Organization #29

Merged
merged 6 commits into from
Oct 10, 2022
Merged

TRAD-259 Organization #29

merged 6 commits into from
Oct 10, 2022

Conversation

aspin
Copy link

@aspin aspin commented Oct 8, 2022

reorganizes endpoint after discussion with Daniel. Highlights:

  • get rid of /amm section
  • instead, move everything into either /market or /trade
  • add some general sectioning in swagger to indicate AMMs vs Orderbooks

kdonthi
kdonthi suggested changes Oct 8, 2022
View reviewed changes
Copy link
Contributor

@kdonthi kdonthi left a comment

Choose a reason for hiding this comment

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

I am getting a bit confused by these tags.

Could we do slightly different tags where the first part is [Amm, Orderbook] and the second part is [Info, Trade]?

@aspin
Copy link
Author

aspin commented Oct 9, 2022

I am getting a bit confused by these tags.

Could we do slightly different tags where the first part is [Amm, Orderbook] and the second part is [Info, Trade]?

see https://github.com/bloXroute-Labs/solana-trader-api/pull/223#issuecomment-1272208449
I'm not sure if it's possible to do multiple tags. Let me see...

Even if it is it'd result in endpoints in multiple sections in Swagger, which I don't think is very intuitive

@aspin aspin requested a review from kdonthi October 10, 2022 18:11
@aspin
Copy link
Author

aspin commented Oct 10, 2022

Spent some time on this. Needed to manually generate a global tag list, but got it all working.

@aspin
Copy link
Author

aspin commented Oct 10, 2022

Screen Shot 2022-10-10 at 1 19 04 PM

looks like this now. Some endpoints will be in multiple sections

@aspin aspin merged commit a491c3c into develop Oct 10, 2022
@aspin aspin deleted the TRAD-259/organization branch October 10, 2022 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yobloxroute yobloxroute Awaiting requested review from yobloxroute

@tinystarinagalaxy tinystarinagalaxy Awaiting requested review from tinystarinagalaxy

@kdonthi kdonthi Awaiting requested review from kdonthi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aspin @kdonthi