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

chore(ux): improve error message when attaching without subject artifact #1430

Merged
merged 23 commits into from
Jul 4, 2024

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Jun 24, 2024

What this PR does / why we need it:
This PR improves error message when the oras attach is run with only one argument, which might be an invalid artifact subject reference or mistakenly used as a to-be-uploaded file path.

Before

> oras attach --artifact-type test/example $ARTIFACT ./layers 
Error: neither file nor annotation provided in the command
Usage: oras attach [flags] --artifact-type=<type> <name>{:<tag>|@<digest>} <file>[:<layer_media_type>] [...]
To attach to an existing artifact, please provide files via argument or annotations via flag "--annotation". Run "oras attach -h" for more options and examples

After

> oras attach --artifact-type test/example $ARTIFACT ./layers 
Error: "./layers": no tag or digest specified
Usage: oras attach [flags] --artifact-type=<type> <name>{:<tag>|@<digest>} <file>[:<layer_media_type>] [...]
Have you specified an artifact reference to attach to? Please specify a reference in the form of "<name>:<tag>" or "<name>@<digest>". Run "oras attach -h" for more options and examples

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1424, #1404

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.41%. Comparing base (e7ffb65) to head (e1163c7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1430      +/-   ##
==========================================
+ Coverage   85.38%   85.41%   +0.02%     
==========================================
  Files         108      108              
  Lines        3818     3825       +7     
==========================================
+ Hits         3260     3267       +7     
- Misses        333      334       +1     
+ Partials      225      224       -1     

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

@qweeah qweeah marked this pull request as ready for review June 24, 2024 01:06
@wangxiaoxuan273
Copy link
Contributor

LGTM but I'm not a maintainer

Signed-off-by: Billy Zha <[email protected]>
Copy link
Member

@TerryHowe TerryHowe left a comment

Choose a reason for hiding this comment

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

/lgtm

cmd/oras/internal/errors/errors.go Show resolved Hide resolved
cmd/oras/internal/errors/errors.go Outdated Show resolved Hide resolved
cmd/oras/internal/errors/errors.go Outdated Show resolved Hide resolved
cmd/oras/internal/errors/errors.go Outdated Show resolved Hide resolved
cmd/oras/internal/errors/errors.go Outdated Show resolved Hide resolved
cmd/oras/internal/errors/errors.go Outdated Show resolved Hide resolved
cmd/oras/root/attach.go Outdated Show resolved Hide resolved
cmd/oras/root/attach.go Outdated Show resolved Hide resolved
Copy link
Member

@FeynmanZhou FeynmanZhou left a comment

Choose a reason for hiding this comment

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

Just one suggested error message for this case

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM with suggestions

cmd/oras/root/attach.go Outdated Show resolved Hide resolved
Copy link
Member

@FeynmanZhou FeynmanZhou left a comment

Choose a reason for hiding this comment

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

LGTM

@qweeah qweeah merged commit a4a4b33 into oras-project:main Jul 4, 2024
8 checks passed
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.

improve error message of oras attach when subject artifact is not specified
5 participants