Skip to content

fix: redirect log outputs to stderr instead of stdout#3658

Merged
rina23q merged 3 commits intothin-edge:mainfrom
rina23q:fix/3646/spinner-output-should-go-to-stderr
Jun 3, 2025
Merged

fix: redirect log outputs to stderr instead of stdout#3658
rina23q merged 3 commits intothin-edge:mainfrom
rina23q:fix/3646/spinner-output-should-go-to-stderr

Conversation

@rina23q
Copy link
Copy Markdown
Member

@rina23q rina23q commented May 30, 2025 •
edited
Loading

Proposed changes

  • The first commit changes only the output of Spinner. That has to be stderr as commented here.
  • The second and third commits change the output target of some messages to stderr. They look more like log outputs, so stderr is more appropriate than stdout.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3646

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@rina23q rina23q temporarily deployed to Test Pull Request May 30, 2025 16:56 — with GitHub Actions Inactive
@rina23q rina23q changed the title Fix/3646/spinner output should go to stderr fix: redirect log outputs to stderr instead of stdout May 30, 2025
@rina23q rina23q mentioned this pull request May 30, 2025
Closed
7 tasks
@codecov
Copy link
Copy Markdown

codecov bot commented May 30, 2025

Codecov Report

Attention: Patch coverage is 11.11111% with 24 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/core/tedge/src/cli/connect/command.rs 0.00% 13 Missing ⚠️
crates/core/tedge/src/cli/log.rs 33.33% 5 Missing and 1 partial ⚠️
crates/core/tedge/src/cli/refresh_bridges.rs 0.00% 4 Missing ⚠️
crates/core/tedge/src/cli/reconnect/command.rs 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 30, 2025 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
641 0 3 641 100 1h55m48.650605s

@rina23q rina23q marked this pull request as draft May 30, 2025 17:56
@rina23q
fix: redirect spinner output to stderr instead of stdout
36d9143 
The spinner should not interfere with stdout content,
so it now writes to stderr as intended.

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
@rina23q rina23q force-pushed the fix/3646/spinner-output-should-go-to-stderr branch from bbe5f9d to 7a9b7b4 Compare June 2, 2025 14:02
@rina23q rina23q had a problem deploying to Test Pull Request June 2, 2025 14:02 — with GitHub Actions Failure
@rina23q rina23q temporarily deployed to Test Pull Request June 2, 2025 15:01 — with GitHub Actions Inactive
@rina23q rina23q temporarily deployed to Test Pull Request June 2, 2025 17:09 — with GitHub Actions Inactive
@rina23q rina23q temporarily deployed to Test Pull Request June 2, 2025 17:41 — with GitHub Actions Inactive
didier-wenzek
didier-wenzek reviewed Jun 3, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

I will be happy to approve this PR, once fixed the system test that is still incorrectly expecting a log entry on stdout instead of stderr.

@rina23q rina23q force-pushed the fix/3646/spinner-output-should-go-to-stderr branch from fc5b790 to d85bd84 Compare June 3, 2025 12:02
@rina23q rina23q temporarily deployed to Test Pull Request June 3, 2025 12:02 — with GitHub Actions Inactive
@rina23q rina23q marked this pull request as ready for review June 3, 2025 12:04
@rina23q rina23q requested review from a team and reubenmiller as code owners June 3, 2025 12:04
didier-wenzek
didier-wenzek approved these changes Jun 3, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

Approved

@reubenmiller reubenmiller added the theme:cli Theme: cli related topics label Jun 3, 2025
@rina23q rina23q temporarily deployed to Test Pull Request June 3, 2025 15:06 — with GitHub Actions Inactive
rina23q added 2 commits June 3, 2025 17:59
@rina23q
fix: change log output target from stdout to stderr
5d85325 
Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
@rina23q
test: update Execute Remote Access Command to return both stdout and …
5151b48 
…stderr

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
@rina23q rina23q force-pushed the fix/3646/spinner-output-should-go-to-stderr branch from 9295088 to 5151b48 Compare June 3, 2025 17:59
@rina23q rina23q temporarily deployed to Test Pull Request June 3, 2025 17:59 — with GitHub Actions Inactive
@rina23q rina23q enabled auto-merge June 3, 2025 18:00
@rina23q rina23q added this pull request to the merge queue Jun 3, 2025
Merged via the queue into thin-edge:main with commit 4c0858d Jun 3, 2025
34 checks passed
@rina23q rina23q deleted the fix/3646/spinner-output-should-go-to-stderr branch June 3, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reubenmiller reubenmiller reubenmiller approved these changes

@didier-wenzek didier-wenzek didier-wenzek approved these changes

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

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

Assignees

No one assigned

Labels

theme:cli Theme: cli related topics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rina23q @reubenmiller @didier-wenzek