Skip to content

fix: OCI store resolve returns full descriptor#468

Merged
Wwwsylvia merged 1 commit into
oras-project:mainfrom
opcr-io:resolve_returns_full_descriptor
Mar 23, 2023
Merged

fix: OCI store resolve returns full descriptor#468
Wwwsylvia merged 1 commit into
oras-project:mainfrom
opcr-io:resolve_returns_full_descriptor

Conversation

@oanatmaria

@oanatmaria oanatmaria commented Mar 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Based on discussion on #457, this PR returns a full descriptor on Resolve when the method it's called by tag and it returns a plain descriptor when the method it's called by digest.

@oanatmaria oanatmaria closed this Mar 21, 2023
@oanatmaria oanatmaria reopened this Mar 21, 2023
@oanatmaria oanatmaria changed the title OCI store resolve returns full descriptor fix: OCI store resolve returns full descriptor Mar 21, 2023
shizhMSFT
shizhMSFT approved these changes Mar 22, 2023
View reviewed changes

@shizhMSFT shizhMSFT 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

Wwwsylvia
Wwwsylvia reviewed Mar 22, 2023
View reviewed changes

@Wwwsylvia Wwwsylvia left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @oanatmaria , thanks for the contribution! Do you want to update the same for ReadOnlyStore as well?

Comment thread content/oci/oci_test.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci_test.go Outdated
@oanatmaria

oanatmaria commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

Hi @oanatmaria , thanks for the contribution! Do you want to update the same for ReadOnlyStore as well?

Hi @Wwwsylvia,

Thank you for the review. I've made the change for the readonlystore as well.

Wwwsylvia
Wwwsylvia reviewed Mar 23, 2023
View reviewed changes
Comment thread content/oci/oci_test.go Outdated
Comment thread content/oci/oci_test.go Outdated
@Wwwsylvia

Wwwsylvia commented Mar 23, 2023

Copy link
Copy Markdown
Member

Also wondering if we should call out the behavior difference between resolving by tag and resolving by digest in the doc comment of the Resolve() function? @oanatmaria What do you think?

@oanatmaria

oanatmaria commented Mar 23, 2023

Copy link
Copy Markdown
Contributor Author

Also wondering if we should call out the behavior difference between resolving by tag and resolving by digest in the doc comment of the Resolve() function? @oanatmaria What do you think?

I've updated the comments. Please let me know if they are correctly formulated.

Wwwsylvia
Wwwsylvia approved these changes Mar 23, 2023
View reviewed changes

@Wwwsylvia Wwwsylvia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread content/oci/oci.go Outdated
OCI store resolve returns full descriptor
3239ae6 
Signed-off-by: oanatmaria <oana@aserto.com>
@Wwwsylvia Wwwsylvia merged commit e8225cb into oras-project:main Mar 23, 2023
@Wwwsylvia

Wwwsylvia commented Mar 23, 2023

Copy link
Copy Markdown
Member

Merged. Thank you! @oanatmaria

@shizhMSFT shizhMSFT mentioned this pull request May 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wwwsylvia Wwwsylvia Wwwsylvia approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@sajayantony sajayantony Awaiting requested review from sajayantony

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

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

@oanatmaria @Wwwsylvia @shizhMSFT