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

Fix "manifest list" typos for manifest annotations #484

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

zhouhao3
Copy link

@zhouhao3 zhouhao3 commented Dec 8, 2016

Signed-off-by: zhouhao zhouhao@cn.fujitsu.com

@zhouhao3 zhouhao3 force-pushed the json-test branch 4 times, most recently from 1c10a05 to 3afd985 Compare January 16, 2017 06:25
@zhouhao3
Copy link
Author

@stevvooe @vbatts @jonboulle PTAL

@stevvooe
Copy link
Contributor

@q384566678 I have no clue what this PR is doing. Please expand on "Modify the error message". What is this fixing?

@zhouhao3
Copy link
Author

@stevvooe Should be a writing error, manifest the annotations field id should be manifest/annotations instead of manifestlist-list/annotations.

@stevvooe
Copy link
Contributor

@q384566678 What should be writing an error? Do you have an example of what happens with and without this patch?

@zhouhao3
Copy link
Author

@stevvooe Uh, if not revised, then there should not be any mistakes. I think here mainly belongs to writing errors.

@zhouhao3
Copy link
Author

@stevvooe @vbatts @philips I have updated this PR, I think these two should be modified. It can be seen from the specification that manifest list and image manifest are different in Annotations.PTAL

@zhouhao3 zhouhao3 force-pushed the json-test branch 2 times, most recently from 0b1750c to 4bf977f Compare February 13, 2017 10:05
@stevvooe
Copy link
Contributor

@q384566678 Are you saying that this is missing? Could you provide an example of the output before and after this change?

@zhouhao3 zhouhao3 force-pushed the json-test branch 11 times, most recently from 405a5fc to 65c7095 Compare February 14, 2017 06:04
@zhouhao3
Copy link
Author

@stevvooe Uh......I mean that in my two changes, the manifest and manifest list confused, if not changed, then it will not be wrong, because it is only the role of the description, not the specific data call.

@wking
Copy link
Contributor

wking commented Feb 14, 2017 via email

@wking
Copy link
Contributor

wking commented Feb 14, 2017 via email

@zhouhao3 zhouhao3 changed the title Modify the error message Fix "manifest list" typos for manifest annotations Feb 15, 2017
@zhouhao3
Copy link
Author

The PR subject and commit message summary are pretty bad though ;).

My title does have a problem.updated.thanks.

@wking
Copy link
Contributor

wking commented Feb 15, 2017 via email

Signed-off-by: zhouhao <zhouhao@cn.fujitsu.com>
@vbatts
Copy link
Member

vbatts commented Apr 3, 2017

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

jonboulle commented Apr 4, 2017

lgtm

Approved with PullApprove

@jonboulle jonboulle merged commit 3ff2369 into opencontainers:master Apr 4, 2017
@zhouhao3 zhouhao3 deleted the json-test branch April 5, 2017 01:03
@vbatts vbatts mentioned this pull request May 19, 2017
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