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

🐛 (cli): print listApps output only once #9127

Merged
merged 1 commit into from
Feb 10, 2025
Merged

🐛 (cli): print listApps output only once #9127

merged 1 commit into from
Feb 10, 2025
+7 −9

Conversation

francois-guerin-ledger
Copy link
Contributor

@francois-guerin-ledger francois-guerin-ledger commented Feb 5, 2025 •
edited
Loading

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    • ...

📝 Description

The result of the listApps command is currently printed twice:

  • in the lines removed in this commit
  • in the next callback function of the main subscriber (cf

    ledger-live/apps/cli/src/cli.ts

    Lines 84 to 86 in 25bea5f

    next: log => {
    if (log !== undefined) console.log(log);
    },
    )

❓ 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)

@francois-guerin-ledger francois-guerin-ledger requested a review from a team February 5, 2025 17:11
@francois-guerin-ledger francois-guerin-ledger self-assigned this Feb 5, 2025
@vercel Vercel
Copy link

vercel bot commented Feb 5, 2025 •
edited
Loading

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

4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-github-bot ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:55am
native-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:55am
react-ui-storybook ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:55am
web-tools ⬜️ Ignored (Inspect) Visit Preview Feb 10, 2025 11:55am

@francois-guerin-ledger francois-guerin-ledger force-pushed the fix/no-issue-listApps-printed-twice branch from 3973bba to 66f7e3e Compare February 5, 2025 17:15
@francois-guerin-ledger francois-guerin-ledger requested review from a team and removed request for a team February 7, 2025 09:27
Wozacosta
Wozacosta approved these changes Feb 10, 2025
View reviewed changes
Copy link
Contributor

@Wozacosta Wozacosta left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@francois-guerin-ledger francois-guerin-ledger force-pushed the fix/no-issue-listApps-printed-twice branch from 66f7e3e to 739f45d Compare February 10, 2025 11:23
@francois-guerin-ledger
fix (cli): print listApps output only once
f502aa5 
The result of the `listApps` command is currently printed twice:
- in the lines removed in this commit
- in the `next` callback function of the main subscriber

Signed-off-by: François Guérin <[email protected]>
@francois-guerin-ledger francois-guerin-ledger force-pushed the fix/no-issue-listApps-printed-twice branch from 739f45d to f502aa5 Compare February 10, 2025 11:54
@francois-guerin-ledger francois-guerin-ledger added this pull request to the merge queue Feb 10, 2025
Merged via the queue into develop with commit 1bef14e Feb 10, 2025
50 of 51 checks passed
@francois-guerin-ledger francois-guerin-ledger deleted the fix/no-issue-listApps-printed-twice branch February 10, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wozacosta Wozacosta Wozacosta approved these changes

Assignees

@francois-guerin-ledger francois-guerin-ledger

Labels
cli
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@francois-guerin-ledger @Wozacosta