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

refactor!: modernize Registry.Remote #94

Merged
merged 10 commits into from
Jan 9, 2024

Conversation

shizhMSFT
Copy link
Contributor

@shizhMSFT shizhMSFT commented Jan 4, 2024

What this PR does / why we need it

  • Modernize all classes under OrasProject.Oras.Registry.Remote with latest dotnet / C# features.
  • Achieves 0 errors, 0 warnings.

Which issue(s) this PR resolves / fixes

Resolve partially #91

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?

Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

Attention: 126 lines in your changes are missing coverage. Please review.

Comparison is base (b6149d4) 71.32% compared to head (d2387be) 70.90%.

❗ Current head d2387be differs from pull request most recent head 2589326. Consider uploading reports for the commit 2589326 to get more accurate results

Files Patch % Lines
...s/Registry/Remote/HttpResponseMessageExtensions.cs 64.28% 34 Missing and 11 partials ⚠️
src/OrasProject.Oras/Registry/Remote/BlobStore.cs 57.89% 17 Missing and 7 partials ⚠️
.../OrasProject.Oras/Registry/Remote/ManifestStore.cs 72.60% 12 Missing and 8 partials ⚠️
src/OrasProject.Oras/Registry/Remote/Repository.cs 80.51% 11 Missing and 4 partials ⚠️
src/OrasProject.Oras/Registry/Remote/Registry.cs 83.67% 4 Missing and 4 partials ⚠️
src/OrasProject.Oras/Registry/Remote/UriFactory.cs 87.50% 4 Missing and 2 partials ⚠️
src/OrasProject.Oras/Registry/Reference.cs 70.58% 5 Missing ⚠️
src/OrasProject.Oras/Extensions.cs 0.00% 2 Missing ⚠️
...as/Registry/Remote/Auth/HttpClientWithBasicAuth.cs 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
- Coverage   71.32%   70.90%   -0.43%     
==========================================
  Files          35       33       -2     
  Lines        1001      921      -80     
  Branches       97      105       +8     
==========================================
- Hits          714      653      -61     
+ Misses        237      215      -22     
- Partials       50       53       +3     

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

Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM

if (remoteReference.Registry != _opts.Reference.Registry || remoteReference.Repository != _opts.Reference.Repository)
{
throw new InvalidReferenceException(
$"mismatch between received {JsonSerializer.Serialize(remoteReference)} and expected {JsonSerializer.Serialize(_opts.Reference)}");
Copy link
Member

Choose a reason for hiding this comment

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

nit: Mismatch?

@Wwwsylvia
Copy link
Member

Achieves 0 errors, 0 warnings.

This is cool! 🥳

@Wwwsylvia Wwwsylvia merged commit 6523fd6 into oras-project:main Jan 9, 2024
5 checks passed
@shizhMSFT shizhMSFT deleted the refactor_remote branch January 10, 2024 03:11
@shizhMSFT shizhMSFT mentioned this pull request Jan 15, 2024
3 tasks
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.

2 participants