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

[Content migration] - Asset Transfer API #57

Merged
merged 2 commits into from
Sep 27, 2024
Merged

Conversation

0xLucca
Copy link
Collaborator

@0xLucca 0xLucca commented Sep 25, 2024

This PR incorporates content from the polkadot-ecosystem-docs-draft repository.

Migrated content:

  • Asset Transfer API

@0xLucca 0xLucca requested a review from nhussein11 September 25, 2024 17:14

const assetsApi = new AssetTransferApi(api, specName, safeXcmVersion);

// Your code using assetsApi goes here
Copy link
Collaborator

@nhussein11 nhussein11 Sep 26, 2024

Choose a reason for hiding this comment

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

Some snippets use assetsApi and other assetApi, stick to one way

Also, I find kind of weird this comment:
// Your code using assetsApi goes here
But is nitpicking at the end of the day, if you find a better way to express it cool, if not no worries :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It meant the variable called assetsApi that has already been initialized in line 11. Do you think it looks better if we put assetTransferApi ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I get that. My point here was that in some snippets, you initialized the variable in a plural form, and in others, you used assetApi in a singular

@0xLucca 0xLucca requested a review from nhussein11 September 27, 2024 13:35
Copy link
Collaborator

@nhussein11 nhussein11 left a comment

Choose a reason for hiding this comment

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

lgtm!

@nhussein11 nhussein11 added the B0 - Needs Review Pull request is ready for review label Sep 27, 2024
@0xLucca 0xLucca merged commit 0f58a2f into master Sep 27, 2024
@0xLucca 0xLucca deleted the 0xlucca/asset-transfer-api branch September 27, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B0 - Needs Review Pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants