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

Test for RegistryClient.pullManifest() #2555

Merged
merged 18 commits into from
Jul 1, 2020
Merged

Test for RegistryClient.pullManifest() #2555

merged 18 commits into from
Jul 1, 2020

Conversation

louismurerwa
Copy link
Contributor

@louismurerwa louismurerwa commented Jun 30, 2020

Part of #1808

@chanseokoh
Copy link
Member

chanseokoh commented Jun 30, 2020

Also, let's have another pullManifest() test that pulls in a manifest list (instead of a manifest). Here's an example. When using the example manifest list, I believe the type of the returned manifest is V22ManifestListTemplate.

Note that you can find these -Template classes in the com.google.cloud.tools.jib.image.json package.

image

@louismurerwa
Copy link
Contributor Author

@chanseokoh Should I write the pullManifest test that uses a manifest list(instead of a manifest file) as a separate test function?

@chanseokoh
Copy link
Member

Yes, in a separate test.

Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

So far so good. We're right on track.

@louismurerwa louismurerwa requested a review from chanseokoh June 30, 2020 20:00
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

Will wait for the manifest list test.

Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

Looking good. Minor comments.

@chanseokoh
Copy link
Member

I still see some of my nit comments have not been addressed.

Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

LGTM. Merge when you think ready.

@louismurerwa louismurerwa merged commit c867dca into master Jul 1, 2020
@louismurerwa louismurerwa deleted the pullmanifest branch July 1, 2020 16:09
@louismurerwa louismurerwa removed the request for review from loosebazooka August 17, 2020 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants