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

Coin modularization / refactoring for MultiversX #7637

Closed

Conversation

andreibancioiu
Copy link

@andreibancioiu andreibancioiu commented Aug 21, 2024

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
    • added a few new unit tests (more to be added on the re-branding PR)
  • Impact of the changes:
    • features related to MultiversX: transfers of native & custom currencies, staking actions (staking, claiming rewards, etc.)

📝 Description

  • Refactored MultiversX components to match the coin modularization guidelines.
  • Renaming of "Elrond" to "MultiversX" will be applied in a separate PR.
  • Fixed display issue (present in current version of Ledger Live) - regarding the displayed "delegated amount".

❓ Context

  • JIRA or GitHub link:

🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

popenta and others added 27 commits July 29, 2024 15:09
Moved files from ledger-live-common to new package without any modifications
Modified files to match new guidelines and moved hw-app to separate package
…lity

Renamed package to old name for easier integration
Fixed ledger live imports
…ultiversx-2024-08-21

Merge develop into feat/coin-multiversx (2024-08-21)
Copy link

vercel bot commented Aug 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
web-tools ❌ Failed (Inspect) Sep 4, 2024 7:14am
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-docs ⬜️ Ignored (Inspect) Visit Preview Sep 4, 2024 7:14am
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Sep 4, 2024 7:14am
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Sep 4, 2024 7:14am

Copy link

vercel bot commented Aug 21, 2024

@andreibancioiu is attempting to deploy a commit to the LedgerHQ Team on Vercel.

A member of the Team first needs to authorize it.

@andreibancioiu andreibancioiu marked this pull request as ready for review September 2, 2024 14:24
@andreibancioiu andreibancioiu requested a review from a team as a code owner September 2, 2024 14:24
package.json Outdated Show resolved Hide resolved
@Wozacosta
Copy link
Contributor

Wozacosta commented Sep 3, 2024

You'll also need to fix the .unimported file from ledger-live-common,
Here's a commit to cherrypick from: 4321a31

Edit: done, ty ✅

@Wozacosta
Copy link
Contributor

Wozacosta commented Sep 3, 2024

The README check for hw-app-elrond is failing, here is a commit where I've generated one (using pnpm doc)

8939d70

Edit: done, ty ✅

@Wozacosta
Copy link
Contributor

Wozacosta commented Sep 3, 2024

This change here 1891fd6
Will fix this test failing, https://github.com/LedgerHQ/ledger-live/actions/runs/10669818142/job/29574353931.
The ignore pattern wasn't matching with the file (bridge.integration.test.ts) you had

Edit: done, ty ✅

@Wozacosta
Copy link
Contributor

Wozacosta commented Sep 3, 2024

After all those changes I'd recommend rebasing with develop (the Ton token was integrated recently, you might have some minor conflicts). If you have problem with the pnpm-lock.yaml during rebase just add it during conflicts without fixing the merge conflicts, at the end of the rebase remove it, then launch a pnpm i and push the new pnpm-lock.yaml

This should fix the e2e tests that are failing on another chain (cosmos)

@Wozacosta
Copy link
Contributor

Wozacosta commented Sep 4, 2024

Here is the latest commit (I think) we'll need,
It fixes pnpm-lock and new unimportedrc errors f1433c0

The branch had new conflicts, I've created a new one from yours (#7750), rebased and squashed commits to avoid endless conflicts resolution, everything should be good from your end now unless there's code changes required, I'll try to get this one through the finish line

Copy link

There as been no activity on this PR for the last 14 days. Please consider closing this PR.

@github-actions github-actions bot added the Stale label Sep 20, 2024
@github-actions github-actions bot closed this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Has changes in live-common desktop Has changes in LLD fork Pull request base branch comes from a fork. ledgerjs Has changes in the ledgerjs open source libs mobile Has changes in LLM Stale tools Has changes in tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants