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: improving error log for oras push and oras attach when the annotation file is invalid #1026

Merged
merged 13 commits into from
Jul 27, 2023

Conversation

1Shubham7
Copy link
Contributor

@1Shubham7 1Shubham7 commented Jul 19, 2023

What this PR does / why we need it:
If a malformed JSON file is provided via --annotation-file flag, oras used to output
Error: json: cannot unmarshal string into Go value of type map[string]string

PR to improve this to

 error loading manifest annotations from file annotations.json: [Error]. refer to the document at https://oras.land/docs/how_to_guides/manifest_annotations

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 #1021

screen shot of the final version
image

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?

@qweeah qweeah changed the title [feat] : improving error log for oras push and oras attach when the annotation file is invalid feat: improving error log for oras push and oras attach when the annotation file is invalid Jul 19, 2023
@1Shubham7 1Shubham7 marked this pull request as ready for review July 20, 2023 01:28
@qweeah
Copy link
Contributor

qweeah commented Jul 20, 2023

@1Shubham7
Copy link
Contributor Author

1Shubham7 commented Jul 20, 2023

figured it out, it should be fine now. @qweeah can you check it out now.

1Shubham7 and others added 4 commits July 22, 2023 12:49
Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
Signed-off-by: Sajay Antony <sajaya@microsoft.com>
Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
@1Shubham7
Copy link
Contributor Author

@qweeah Please have a look at it now.

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2023

Codecov Report

Merging #1026 (addd92f) into main (9260209) will increase coverage by 0.07%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
+ Coverage   81.21%   81.29%   +0.07%     
==========================================
  Files          57       57              
  Lines        2907     2908       +1     
==========================================
+ Hits         2361     2364       +3     
+ Misses        373      371       -2     
  Partials      173      173              
Impacted Files Coverage Δ
cmd/oras/internal/option/packer.go 90.76% <100.00%> (+0.14%) ⬆️

... and 1 file with indirect coverage changes

fix: move stale into .github/workflows (oras-project#1031)
@1Shubham7
Copy link
Contributor Author

removing "," because of this.
mistake

Signed-off-by: Shubham Singh <116020663+1Shubham7@users.noreply.github.com>
@1Shubham7
Copy link
Contributor Author

image

Co-authored-by: Billy Zha <qweeah@gmail.com>
Signed-off-by: Shubham Singh <116020663+1Shubham7@users.noreply.github.com>
@1Shubham7
Copy link
Contributor Author

Maintainers, I am seeing another issue, some errors end with a period (".") while some don't. I had a discussion with @qweeah . Making commits to deal with this.

Signed-off-by: Shubham Singh <116020663+1Shubham7@users.noreply.github.com>
Co-authored-by: Billy Zha <qweeah@gmail.com>
Signed-off-by: Shubham Singh <116020663+1Shubham7@users.noreply.github.com>
@1Shubham7
Copy link
Contributor Author

1Shubham7 commented Jul 24, 2023

it has multiple errors @qweeah . changing it.
image

Signed-off-by: Shubham Singh <116020663+1Shubham7@users.noreply.github.com>
Signed-off-by: Shubham Singh <116020663+1Shubham7@users.noreply.github.com>
@1Shubham7
Copy link
Contributor Author

@qweeah please have a look now. I think its completed.

@qweeah
Copy link
Contributor

qweeah commented Jul 25, 2023

@toddysm Can you help review and confirm the UX in the screen shot? Thanks cc @FeynmanZhou

Copy link
Contributor

@qweeah qweeah 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 changed the title feat: improving error log for oras push and oras attach when the annotation file is invalid chore: improving error log for oras push and oras attach when the annotation file is invalid Jul 26, 2023
@qweeah qweeah added the ux User experience related label Jul 26, 2023
@qweeah qweeah merged commit 0b6651a into oras-project:main Jul 27, 2023
5 checks passed
shizhMSFT pushed a commit to shizhMSFT/oras that referenced this pull request Aug 3, 2023
… annotation file is invalid (oras-project#1026)

Signed-off-by: Shubham Singh <shubhammahar1306@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ux User experience related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve error log for oras push and oras attach when annotation file is invalid
5 participants