Skip to content

Only return errors when they exists#1285

Merged
giuseppe merged 1 commit intocontainers:mainfrom
rhatdan:main
Jul 13, 2022
Merged

Only return errors when they exists#1285
giuseppe merged 1 commit intocontainers:mainfrom
rhatdan:main

Conversation

@rhatdan
Copy link
Copy Markdown
Member

@rhatdan rhatdan commented Jul 12, 2022

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@rhatdan
Only return errors when they exists
f167ddb 
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
saschagrunert
saschagrunert approved these changes Jul 13, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

/lgtm

@flouthoc
Copy link
Copy Markdown
Collaborator

flouthoc commented Jul 13, 2022 •
edited
Loading

PR LGTM, but i still see issues in c/storage even after taking this commit, higher level tests in buildah and podman starts failing as soon we vendor latest c/storage.

giuseppe
giuseppe approved these changes Jul 13, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe giuseppe merged commit 0b04426 into containers:main Jul 13, 2022
@flouthoc
Copy link
Copy Markdown
Collaborator

flouthoc commented Jul 13, 2022 •
edited
Loading

PR LGTM, but i still see issues in c/storage even after taking this commit, higher level tests in buildah and podman starts failing as soon we vendor latest c/storage.

I was able to resolve my issues after updating c/common it seems error generated from fmt.Errorf does not generates cause for errors.Cause(err) != storage.ErrImageUnknown hence vendor of storage will fail if right c/common is not used. I not sure but maybe good idea to get this in as well containers/image#1607 in final vendor.

Edit: c/image had nothing to do with the issue which i was debugging its just a heads up.

@flouthoc flouthoc mentioned this pull request Jul 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@giuseppe giuseppe giuseppe approved these changes

@saschagrunert saschagrunert saschagrunert approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rhatdan @flouthoc @giuseppe @saschagrunert