Skip to content

feat: add MaybeAssetIdConvert trait for runtime benchmarks#316

Merged
mattsse merged 5 commits intomainfrom
matt/fix-assetid-again
Sep 2, 2021
Merged

feat: add MaybeAssetIdConvert trait for runtime benchmarks#316
mattsse merged 5 commits intomainfrom
matt/fix-assetid-again

Conversation

@mattsse
Copy link
Contributor

@mattsse mattsse commented Sep 1, 2021

Changes

  • Use a feature gated trait to create AssetIds in runtime benchmarks

Tests

cargo t --all-features

Issues

@mattsse mattsse requested a review from clearloop September 1, 2021 11:08
@mattsse mattsse added needs review PR needs reviewing WIP Work in progress - do not review or merge and removed needs review PR needs reviewing labels Sep 1, 2021
@mattsse mattsse marked this pull request as draft September 1, 2021 11:19
clearloop
clearloop approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@clearloop clearloop left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, could this be used for the Acala integration?

@mattsse
Copy link
Contributor Author

mattsse commented Sep 1, 2021

lgtm, could this be used for the Acala integration?

I thought so, but then I realized only if we implement this directly in the CurrencyId primitives pallet which is not ideal, because then pint is included everywhere, I guess we'd have to add another trait type which is also not ideal -.-.

@mattsse mattsse changed the title feat: add MaybeTryFrom trait for runtime benchmarks feat: add MaybeAssetIdConvert trait for runtime benchmarks Sep 2, 2021
@mattsse mattsse marked this pull request as ready for review September 2, 2021 07:43
@mattsse mattsse added needs review PR needs reviewing and removed WIP Work in progress - do not review or merge labels Sep 2, 2021
@mattsse
Copy link
Contributor Author

mattsse commented Sep 2, 2021

This trait will be implemented on the Runtime automatically

@mattsse mattsse requested a review from clearloop September 2, 2021 08:21
@mattsse mattsse merged commit 0229d9d into main Sep 2, 2021
@mattsse mattsse deleted the matt/fix-assetid-again branch September 2, 2021 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@clearloop clearloop clearloop approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs review PR needs reviewing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Drop TryFrom for AssetId

2 participants

@mattsse @clearloop