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

docs: update spl token exchange integration #19573

Merged
merged 2 commits into from
Sep 2, 2021

Conversation

t-nelson
Copy link
Contributor

@t-nelson t-nelson commented Sep 2, 2021

Problem

SPL Token exchange integration docs are outdated. Missing best practices use of ATAs, token balance checks and modern instruction names

Summary of Changes

Update them

aeyakovenko
aeyakovenko previously approved these changes Sep 2, 2021
@aeyakovenko
Copy link
Member

this goes into way more details. are we at the point that we can do the same for tokens?

https://docs.solana.com/integrations/exchange#poll-for-blocks

mvines
mvines previously approved these changes Sep 2, 2021
Copy link
Member

@mvines mvines left a comment

Choose a reason for hiding this comment

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

I made some minor changes to strengthen the language so that nobody can come back in the future with "you said it was optional and something bad happened to me"

docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
@t-nelson
Copy link
Contributor Author

t-nelson commented Sep 2, 2021

this goes into way more details. are we at the point that we can do the same for tokens?

https://docs.solana.com/integrations/exchange#poll-for-blocks

Yeah we reference this section from the SPL Token deposits section

@t-nelson
Copy link
Contributor Author

t-nelson commented Sep 2, 2021

I made some minor changes to strengthen the language so that nobody can come back in the future with "you said it was optional and something bad happened to me"

Good call! We'll inevitably get some push back, but we can handle that on a case by case basis

@mergify mergify bot dismissed stale reviews from aeyakovenko and mvines September 2, 2021 16:58

Pull request has been modified.

@t-nelson t-nelson merged commit f4f14c4 into solana-labs:master Sep 2, 2021
mergify bot pushed a commit that referenced this pull request Sep 2, 2021
* docs: update spl token exchange integration

* Apply suggestions from code review

Co-authored-by: Michael Vines <[email protected]>

Co-authored-by: Michael Vines <[email protected]>
(cherry picked from commit f4f14c4)
@t-nelson t-nelson deleted the docs-spl-xchg-int branch September 2, 2021 17:33
mergify bot added a commit that referenced this pull request Sep 2, 2021
* docs: update spl token exchange integration

* Apply suggestions from code review

Co-authored-by: Michael Vines <[email protected]>

Co-authored-by: Michael Vines <[email protected]>
(cherry picked from commit f4f14c4)

Co-authored-by: Trent Nelson <[email protected]>
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Sep 2, 2021
* docs: update spl token exchange integration

* Apply suggestions from code review

Co-authored-by: Michael Vines <[email protected]>

Co-authored-by: Michael Vines <[email protected]>
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Sep 3, 2021
* docs: update spl token exchange integration

* Apply suggestions from code review

Co-authored-by: Michael Vines <[email protected]>

Co-authored-by: Michael Vines <[email protected]>
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants