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(taro-lnd-link-details): info tab for taro-lnd link #667

Merged
merged 3 commits into from
Feb 11, 2023

Conversation

amovfx
Copy link
Contributor

@amovfx amovfx commented Feb 8, 2023

Description

Added basic taro-lnd peer link info.
Added test for taro-lnd link.
Updated LinkDetails tests for taro nodes.

Steps to Test

  1. Create a network with Taro
  2. Click the link between the taro node and lnd node.

Added basic taro-lnd peer link info.
Added test for taro-lnd link.
Updated LinkDetails tests for taro nodes.
Copy link
Contributor Author

@amovfx amovfx left a comment

Choose a reason for hiding this comment

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

Reviewed. I didn't really have any ideas for the taro lnd link node, so I copied the Peer component.

Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

Overall this works as expected. Just had a few comments on the code and labels.

Thanks for adding the tests to keep the coverage up. 👌

src/components/designer/link/TaroLnd.tsx Outdated Show resolved Hide resolved
src/components/designer/link/TaroLnd.tsx Outdated Show resolved Hide resolved
src/i18n/locales/en-US.json Outdated Show resolved Hide resolved
src/i18n/locales/en-US.json Outdated Show resolved Hide resolved
src/i18n/locales/en-US.json Outdated Show resolved Hide resolved
@amovfx
Copy link
Contributor Author

amovfx commented Feb 11, 2023

Is there an easy way to move this over to draft? I decided to add the change TaroBackend ability to this pr. Or should that be a separate pr?

@jamaljsr
Copy link
Owner

jamaljsr commented Feb 11, 2023

There is a "Convert to draft" link under the Reviewers section of this page.

Definitely create a separate PR. These two features are unrelated, and this PR is pretty much done.

@amovfx
Copy link
Contributor Author

amovfx commented Feb 11, 2023

Okay, thanks for the guidance.

Proper casing on the usePrefixedTranslation
Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

tACK 👌

Great job on the tests.

@jamaljsr jamaljsr merged commit 30ac2af into jamaljsr:feat/add-taro Feb 11, 2023
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.

2 participants