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.

Sign up for GitHub

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

Jump to bottom

fix: OCI store resolve returns full descriptor #468

Merged
merged 1 commit into from
Mar 23, 2023
Merged

fix: OCI store resolve returns full descriptor #468

merged 1 commit into from
Mar 23, 2023

Conversation

oanatmaria
Copy link
Contributor

@oanatmaria oanatmaria commented Mar 21, 2023 •
edited
Loading

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

Wwwsylvia
Wwwsylvia reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment •
edited
Loading

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?

@oanatmaria
Copy link
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
@Wwwsylvia
Copy link
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
Copy link
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
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

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
Copy link
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 sajayantony is a code owner

@SteveLasker SteveLasker Awaiting requested review from SteveLasker SteveLasker 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
@oanatmaria @Wwwsylvia @shizhMSFT