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.

Sign up for GitHub

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

Jump to bottom

rename: Connectors -> LiquidityPools #1493

Merged
merged 3 commits into from Aug 14, 2023
Merged

rename: Connectors -> LiquidityPools #1493

merged 3 commits into from Aug 14, 2023

Conversation

NunoAlexandre
Copy link
Contributor

Closes #1489

Notes

I tried to rename "connectors" to "liquidityPools" 1:1 as much as possible but in certain cases that didn't make a lot of sense. For example, when adding a "known connector", I rephrased that as "instance", since "known liquidity pool" is not quite right in my pov. Happy to be told otherwise.

@NunoAlexandre NunoAlexandre mentioned this pull request Aug 11, 2023
Merged
5 tasks
hieronx
hieronx previously approved these changes Aug 13, 2023
View reviewed changes
Copy link
Contributor

@hieronx hieronx left a comment

Choose a reason for hiding this comment

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

LGTM. I think the instance terminology makes sense!

wischli
wischli reviewed Aug 14, 2023
View reviewed changes
Copy link
Contributor

@wischli wischli left a comment •
edited

Choose a reason for hiding this comment

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

If we don't want to start from scratch on the Liquidity Pools Testnet, we unfortunately need to write a simple migration to map all Connectors prefixed storage items to LiquidityPools.

Apart from that only ridiculous nitpicks from my side which don't need to apply and, of course, lots of praise and love. Thanks for applying this and all the improvements! Much better than before.

Cargo.toml Outdated Show resolved Hide resolved
libs/types/src/domain_address.rs Outdated Show resolved Hide resolved
pallets/liquidity-pools-gateway/routers/Cargo.toml Show resolved Hide resolved
pallets/liquidity-pools-gateway/src/lib.rs Show resolved Hide resolved
pallets/liquidity-pools-gateway/src/lib.rs Show resolved Hide resolved
pallets/liquidity-pools/src/message.rs Show resolved Hide resolved
runtime/common/Cargo.toml Outdated Show resolved Hide resolved
runtime/development/Cargo.toml Outdated Show resolved Hide resolved
runtime/development/src/lib.rs Show resolved Hide resolved
runtime/integration-tests/src/liquidity_pools/pallet/development/tests/liquidity_pools.rs Outdated Show resolved Hide resolved
@wischli wischli added P7-asap Issue should be addressed in the next days. D5-breaks-api Pull request changes public api. Document changes publicly. I6-refactoring Code needs refactoring. crcl-cross-chain Circle cross-chain related. labels Aug 14, 2023
@NunoAlexandre
Copy link
Contributor Author

@wischli thanks for the careful review and raising awareness for specific core bits. I believe to have addressed all of your comments, let me know if I missed any 🏄‍♂️

wischli
wischli previously approved these changes Aug 14, 2023
View reviewed changes
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

LGTM! 🤿

mustermeiszer
mustermeiszer previously approved these changes Aug 14, 2023
View reviewed changes
Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

LGTM!

There are some leftover artifacts in the Liquidity-Pools lib.rs but no blocker.

pallets/liquidity-pools/src/lib.rs Show resolved Hide resolved
pallets/liquidity-pools/src/lib.rs Show resolved Hide resolved
@mustermeiszer mustermeiszer mentioned this pull request Aug 14, 2023
Merged
@NunoAlexandre NunoAlexandre enabled auto-merge (squash) August 14, 2023 08:07
@NunoAlexandre
Copy link
Contributor Author

@mustermeiszer @wischli I had to merge main and resolve conflicts introduced by the precompiles PR which added the precompiles to the old connectors-gateway directory; move that here to liquidity-pools-gateway 👍 appreciate another round and hopefully we can get this in asap

wischli
wischli approved these changes Aug 14, 2023
View reviewed changes
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Re-approving

@NunoAlexandre NunoAlexandre self-assigned this Aug 14, 2023
mustermeiszer
mustermeiszer approved these changes Aug 14, 2023
View reviewed changes
Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

Re-approve

@NunoAlexandre NunoAlexandre merged commit c779c2d into main Aug 14, 2023
11 checks passed
@NunoAlexandre NunoAlexandre deleted the liquidity-pools branch August 15, 2023 06:28
wischli pushed a commit that referenced this pull request Aug 15, 2023
@NunoAlexandre @wischli
rename: Connectors -> LiquidityPools (#1493)
541540f 
* rename: Connectors -> LiquidityPools

* Apply review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wischli wischli wischli approved these changes

@mustermeiszer mustermeiszer mustermeiszer approved these changes

@cdamian cdamian Awaiting requested review from cdamian cdamian is a code owner

@branan branan Awaiting requested review from branan

@mikiquantum mikiquantum Awaiting requested review from mikiquantum

@lemunozm lemunozm Awaiting requested review from lemunozm lemunozm is a code owner

@thea-leake thea-leake Awaiting requested review from thea-leake

@hieronx hieronx Awaiting requested review from hieronx hieronx is a code owner

Assignees

@NunoAlexandre NunoAlexandre

Labels
crcl-cross-chain Circle cross-chain related. D5-breaks-api Pull request changes public api. Document changes publicly. I6-refactoring Code needs refactoring. P7-asap Issue should be addressed in the next days.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rename connectors to liquidity-pools
4 participants
@NunoAlexandre @hieronx @wischli @mustermeiszer