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

feat: allow for registry override #369

Merged
merged 10 commits into from Feb 16, 2024
Merged

feat: allow for registry override #369

merged 10 commits into from Feb 16, 2024

Conversation

bee344
Copy link
Contributor

@bee344 bee344 commented Feb 9, 2024

Description

Updated parseRegistry to allow for overriding entries of the existing registry. This is achieved by adding the optional property overridingRegistry to AssetTransferApiOpts. Now propertyIterator takes an extra optional argument override, a boolean that defines whether we intend to override the registry or not. This value is set on parseRegistry and depends on the presence of the property overridingRegistry in the opts passed to the function. If the chainId passed in overridingRegistry is not present in the registry, it just adds it as if it were an injectedRegistry. If both options are present, it first updates the registry with injectedRegistry and then makes the changes as per overridingRegistry.

@bee344 bee344 changed the title feat: allow for registry override [WIP] feat: allow for registry override Feb 9, 2024
@bee344 bee344 marked this pull request as ready for review February 9, 2024 16:23
Copy link
Contributor

@marshacb marshacb left a comment

Choose a reason for hiding this comment

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

minor nit. LGTM

src/types.ts Outdated Show resolved Hide resolved
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Overall looks amazing! Small nit, I think we should change the name to overrideRegistry. In addition to that we need to update the docs in the README.md that go over the AssetTransferApiOpts.

@bee344 bee344 requested a review from TarikGul February 13, 2024 12:55
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

LGTM, great job :)

@bee344 bee344 merged commit 856a638 into main Feb 16, 2024
6 checks passed
@bee344 bee344 deleted the anp-registry-override branch February 16, 2024 14:42
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/parity-tech-update-for-february/6630/1

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.

None yet

4 participants