Conversation
b-yap
commented
Aug 4, 2023
ebma
reviewed
Aug 4, 2023
ebma
reviewed
Aug 4, 2023
b-yap
added a commit
that referenced
this pull request
Aug 7, 2023
ebma
approved these changes
Aug 7, 2023
Member
ebma
left a comment
There was a problem hiding this comment.
Thanks for adding my requested changes @b-yap.
While at first, it might seem like there is still some code that can be deduplicated, I think it's good the way it is right now. For example, some parameters like the IDs are shared between the Polkadot and Kusama asset hub, but still have separate definitions in runtime/common/src/lib.rs but I think it's better this way because we can't necessarily always assume that this is the case. There could have been differences since the Polkadot and Kusama AssetHub are different parachains after all.
LGTM
83c56da to
ad1ad33
Compare
ebma
approved these changes
Aug 7, 2023
Member
|
The previous CI already passed and we only added comments so I will merge it without waiting. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes task #83
General overview of the changes:
Aside from the tasks mentioned in the ticket, this PR also includes:
runtime/integration-tests/pendulumtoruntime/integration-tests[dev-dependencies](see the updatedCargo.toml)setup.rsandpolkadot_test_net.rsintomock.rstest_macros.rstests.rsto 2:pendulum_tests.rsandamplitude_tests.rsnote: Statemint and Statemine have different issued balances, after performing
XTokens::transfer()in Pendulum and Amplitude. See line 454 to 456 intest_macros.rs.