Skip to content

refactor: create tty copy handler#1485

Merged
TerryHowe merged 8 commits into
oras-project:mainfrom
TerryHowe:tty-copy-handler
Dec 5, 2024
Merged

refactor: create tty copy handler#1485
TerryHowe merged 8 commits into
oras-project:mainfrom
TerryHowe:tty-copy-handler

Conversation

@TerryHowe

@TerryHowe TerryHowe commented Aug 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

What this PR does / why we need it:

Create tty copy handler with new StartTracking and StopTracking methods.

Partial: #1542
Closes: #1548

@codecov

codecov Bot commented Aug 20, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 83.92857% with 9 lines in your changes missing coverage. Please review.

Project coverage is 84.07%. Comparing base (0998ba4) to head (9c73682).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/oras/internal/display/status/tty.go 78.12% 5 Missing and 2 partials ⚠️
cmd/oras/root/cp.go 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1485      +/-   ##
==========================================
+ Coverage   83.98%   84.07%   +0.08%     
==========================================
  Files         119      119              
  Lines        5315     5325      +10     
==========================================
+ Hits         4464     4477      +13     
+ Misses        604      602       -2     
+ Partials      247      246       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TerryHowe TerryHowe force-pushed the tty-copy-handler branch 7 times, most recently from 9676775 to dca91a9 Compare August 22, 2024 13:32
@github-actions

github-actions Bot commented Nov 12, 2024

Copy link
Copy Markdown

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions Bot added the stale Inactive issues or pull requests label Nov 12, 2024
@TerryHowe TerryHowe removed the stale Inactive issues or pull requests label Nov 12, 2024
@TerryHowe TerryHowe mentioned this pull request Nov 27, 2024
Closed
6 tasks
@wangxiaoxuan273

wangxiaoxuan273 commented Nov 28, 2024

Copy link
Copy Markdown
Contributor

Thanks for the contribution @TerryHowe! We'll review it ASAP.

wangxiaoxuan273
wangxiaoxuan273 reviewed Nov 29, 2024
View reviewed changes
Comment thread cmd/oras/root/cp.go
wangxiaoxuan273
wangxiaoxuan273 reviewed Nov 29, 2024
View reviewed changes
Comment thread cmd/oras/internal/display/status/interface.go Outdated
qweeah
qweeah requested changes Dec 3, 2024
View reviewed changes

@qweeah qweeah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The overall implementation looks good. Just need to improve tty tests on validating more detailed content.

Comment thread cmd/oras/internal/display/status/interface.go Outdated
Comment thread cmd/oras/internal/display/status/tty.go Outdated
Comment thread cmd/oras/internal/display/status/tty.go Outdated
Comment thread cmd/oras/internal/display/status/tty.go Outdated
Comment thread cmd/oras/internal/display/status/text.go Outdated
Comment thread cmd/oras/root/cp.go Outdated
Comment thread cmd/oras/root/cp.go
Comment thread cmd/oras/internal/display/status/tty_console_test.go Outdated
qweeah
qweeah reviewed Dec 3, 2024
View reviewed changes

@qweeah qweeah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the proposed design of tracking is better than what we have for now. We can further apply it to other commands.

shizhMSFT
shizhMSFT reviewed Dec 3, 2024
View reviewed changes
Comment thread cmd/oras/internal/display/status/tty.go Outdated
Comment thread cmd/oras/internal/display/status/tty.go Outdated
Comment thread cmd/oras/internal/display/status/tty.go Outdated
Comment thread cmd/oras/internal/display/status/tty.go Outdated
@TerryHowe TerryHowe force-pushed the tty-copy-handler branch 2 times, most recently from b7a2ecb to 779e5d8 Compare December 4, 2024 00:38
TerryHowe
TerryHowe commented Dec 4, 2024
View reviewed changes
Comment thread cmd/oras/root/cp_test.go Outdated
TerryHowe
TerryHowe commented Dec 4, 2024
View reviewed changes
Comment thread cmd/oras/root/cp_test.go Outdated
@TerryHowe TerryHowe force-pushed the tty-copy-handler branch 2 times, most recently from 1f71707 to c939b42 Compare December 4, 2024 02:18
qweeah
qweeah reviewed Dec 4, 2024
View reviewed changes
Comment thread cmd/oras/internal/display/status/tty_console_test.go Outdated
qweeah
qweeah reviewed Dec 4, 2024
View reviewed changes
Comment thread cmd/oras/internal/display/status/tty_console_test.go Outdated
qweeah
qweeah reviewed Dec 4, 2024
View reviewed changes
Comment thread cmd/oras/internal/display/status/tty_console_test.go Outdated
qweeah
qweeah reviewed Dec 4, 2024
View reviewed changes
Comment thread cmd/oras/root/cp.go Outdated
TerryHowe and others added 8 commits December 4, 2024 03:46
@TerryHowe
refactor: create tty copy handler
8243cc5 
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
@TerryHowe
fix: address review feedback
d921ff3 
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
@TerryHowe
fix: apply test recommendations
57e44bc 
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
@qweeah @TerryHowe
test(ut): enhance onMounted test
0fd8fa0 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah @TerryHowe
test(unit): fix failed case
124b703 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@TerryHowe
Update cmd/oras/root/cp_test.go
b6dffba 
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
@TerryHowe
Update cmd/oras/root/cp_test.go
7a16830 
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
@TerryHowe
fix: broken tty tests
9c73682 
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
qweeah
qweeah approved these changes Dec 4, 2024
View reviewed changes

@qweeah qweeah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@TerryHowe TerryHowe merged commit 363dc2d into oras-project:main Dec 5, 2024
@qweeah qweeah mentioned this pull request Dec 9, 2024
Merged
@Wwwsylvia Wwwsylvia mentioned this pull request Sep 3, 2025
Merged
6 tasks
@TerryHowe TerryHowe deleted the tty-copy-handler branch September 14, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shizhMSFT shizhMSFT shizhMSFT left review comments

@qweeah qweeah qweeah approved these changes

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

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

+1 more reviewer

@wangxiaoxuan273 wangxiaoxuan273 wangxiaoxuan273 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

refactor oras cp to use handlers for command output

4 participants

@TerryHowe @wangxiaoxuan273 @qweeah @shizhMSFT