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

compatibility_test.go:change manifest to manifestlist #509

Merged
merged 1 commit into from
Jan 25, 2017

Conversation

zhouhao3
Copy link

Signed-off-by: zhouhao [email protected]

@zhouhao3
Copy link
Author

@stevvooe @vbatts @jbouzane @jonboulle PTAL

@@ -110,14 +110,14 @@ func TestBackwardsCompatibilityManifestList(t *testing.T) {
fail: false,
},
} {
sum := sha256.Sum256([]byte(tt.manifest))
sum := sha256.Sum256([]byte(tt.manifestlist))
Copy link
Contributor

Choose a reason for hiding this comment

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

While we're in here, let's start using the digest package to perform these kinds of operations.

Copy link
Author

Choose a reason for hiding this comment

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

The digest open is manifeslist, so I think there is nothing wrong with the use of manifestlist here.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is not what I said. I meant that if you're in the process of cleaning up, it would be better to use the go-digest package.

I've filed #513 to take care this, so don't worry about it.

@vbatts
Copy link
Member

vbatts commented Jan 18, 2017

looks good. please rebase

@RobDolinMS
Copy link
Collaborator

@stevvooe Are you OK with @q384566678's response?

@stevvooe
Copy link
Contributor

@RobDolinMS No but I guess we can merge anyways. I've filed #513 to handle the concern.

@zhouhao3
Copy link
Author

updated. PTAL

@stevvooe
Copy link
Contributor

stevvooe commented Jan 19, 2017

LGTM

Approved with PullApprove

1 similar comment
@philips
Copy link
Contributor

philips commented Jan 25, 2017

LGTM

Approved with PullApprove

@philips philips merged commit 3751369 into opencontainers:master Jan 25, 2017
@zhouhao3 zhouhao3 deleted the compatibility-test branch February 6, 2017 01:16
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.

5 participants