Skip to content

feat: remove empty artifact CLI message and show hints for pulling OCI image layout#1157

Merged
sajayantony merged 1 commit into
oras-project:mainfrom
sajayantony:pull-logs
Nov 2, 2023
Merged

feat: remove empty artifact CLI message and show hints for pulling OCI image layout#1157
sajayantony merged 1 commit into
oras-project:mainfrom
sajayantony:pull-logs

Conversation

@sajayantony

@sajayantony sajayantony commented Oct 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
Update the message for oras pull of an image to something actionable -

The current outputs looks like this

❯ oras pull localhost:5000/hello-world:latest
Downloaded empty artifact
Pulled [registry] localhost:5000/hello-world:latest
Digest: sha256:7e9b6e7ba2842c91cf49f3e214d04a7a496f8214356f41d81a6e6dcad11f11e3

The PR modifies this to

❯ go run ./cmd/oras  pull localhost:5000/hello-world:latest
Skipped pulling layers without "org.opencontainers.image.title"
Use 'oras copy localhost:5000/hello-world:latest --to-oci-layout <layout-dir>' to pull all layers.

Also changed to ✓ character for checkmark.

@sajayantony sajayantony marked this pull request as draft October 31, 2023 22:05
@sajayantony sajayantony marked this pull request as ready for review October 31, 2023 22:49
@sajayantony

sajayantony commented Nov 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Open to rewording this

Skipped pulling layers without "org.opencontainers.image.title"

Another option would be

Skipped pulling layers without annotation value for "org.opencontainers.image.title"

qweeah
qweeah reviewed Nov 1, 2023
View reviewed changes
Comment thread cmd/oras/root/pull.go
qweeah
qweeah reviewed Nov 2, 2023
View reviewed changes
Comment thread cmd/oras/root/pull.go Outdated
@qweeah qweeah changed the title Removed empty artifact CLI message feat: remove empty artifact CLI message and show hints for pulling OCI image layout Nov 2, 2023
qweeah
qweeah approved these changes Nov 2, 2023
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 with suggestions:
Action prompt Pulled can be formatted into the same length with Skipped
image to make output aligned.

@sajayantony
Removed empty artifact CLI message
14833b8 
Signed-off-by: Sajay Antony <sajaya@microsoft.com>
@codecov-commenter

codecov-commenter commented Nov 2, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1157 (14833b8) into main (9ec4f1b) will increase coverage by 0.08%.
The diff coverage is 87.50%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main    #1157      +/-   ##
==========================================
+ Coverage   79.85%   79.94%   +0.08%     
==========================================
  Files          65       65              
  Lines        3520     3520              
==========================================
+ Hits         2811     2814       +3     
+ Misses        494      492       -2     
+ Partials      215      214       -1
Files Coverage Δ
cmd/oras/internal/display/progress/status.go 95.08% <100.00%> (ø)
cmd/oras/root/pull.go 73.91% <86.66%> (+1.44%) ⬆️

@sajayantony sajayantony merged commit f41d56d into oras-project:main Nov 2, 2023
@qweeah qweeah mentioned this pull request Feb 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qweeah qweeah qweeah approved these changes

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

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sajayantony @codecov-commenter @qweeah