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

bugfix: walk function exit incorrectly #229

Merged
merged 1 commit into from
Aug 31, 2016
Merged

bugfix: walk function exit incorrectly #229

merged 1 commit into from
Aug 31, 2016

Conversation

xiekeyang
Copy link
Contributor

@xiekeyang xiekeyang commented Aug 31, 2016

Sorry it is my previous fault by #201 .
That patch leads the walk function exit incorrectly.
it is because that function filepatch.Walk() must not return error when meeting path mismatch error. Instead it should return nil. And the checking should be done in the end of walk.
Here I fix the bug in this patch. I retrieve old assertion block. I've run create , validate and unpack command according to the command manual successfully.
I will improve the mechanism gracefully.
PTAL, Thanks lots!
Signed-off-by: xiekeyang xiekeyang@huawei.com

@xiekeyang
Copy link
Contributor Author

xiekeyang commented Aug 31, 2016

BTW, maybe the ci test is some insufficient. We had better to improve the ci test (maybe circleci is nice) , to make sure being able to check the main functionalities.

case nil:
return nil, fmt.Errorf("%s: config not found", cpath)
case errEOW:
// found, continue below
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this comment

@jonboulle
Copy link
Contributor

two small comments then lgtm

@xiekeyang
Copy link
Contributor Author

@jonboulle modified. PTAL, thanks.

Signed-off-by: xiekeyang <xiekeyang@huawei.com>
@jonboulle
Copy link
Contributor

jonboulle commented Aug 31, 2016

LGTM

Approved with PullApprove

@wking
Copy link
Contributor

wking commented Aug 31, 2016

On Wed, Aug 31, 2016 at 04:21:21AM -0700, xiekeyang wrote:

BTW, maybe the ci test is some insufficient. I'd like to improve the
ci test ASAP, to make sure check the main functionality.

Some thoughts on testing here 1. I'm happy to write up integration
and/or unit tests using whatever harness the maintainers want ;).

@vbatts
Copy link
Member

vbatts commented Aug 31, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit dc8d653 into opencontainers:master Aug 31, 2016
@xiekeyang xiekeyang deleted the bugfix branch September 1, 2016 00:36
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.

4 participants