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

Implemented more thiserror for libcontainer (Part 2) #1881

Merged
merged 8 commits into from
May 8, 2023

Conversation

yihuaf
Copy link
Collaborator

@yihuaf yihuaf commented May 4, 2023

Implemented more thiserror for the libcontainer. This PR covers most of the low hanging fruits with smaller crates. Note, I decided to add a new syscall error wrapper that wraps the nix::Error, std::io::Error, and our own syscall crate. This aims to simplify where we call syscalls from 3 different sources and can simplify/unify many errors.

CC @squili PTAL

Part of #1377

@yihuaf yihuaf requested review from utam0k and a team May 4, 2023 19:32
@codecov-commenter
Copy link

codecov-commenter commented May 4, 2023

Codecov Report

Merging #1881 (433b291) into main (5f31428) will decrease coverage by 0.15%.
The diff coverage is 19.76%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1881      +/-   ##
==========================================
- Coverage   67.71%   67.56%   -0.15%     
==========================================
  Files         124      125       +1     
  Lines       14205    14199       -6     
==========================================
- Hits         9619     9594      -25     
- Misses       4586     4605      +19     

@yihuaf yihuaf self-assigned this May 5, 2023
@yihuaf yihuaf force-pushed the yihuaf/more-error branch from 5970ed6 to 0a9234f Compare May 5, 2023 04:15
yihuaf added 2 commits May 5, 2023 18:47
Signed-off-by: yihuaf <[email protected]>
@yihuaf yihuaf force-pushed the yihuaf/more-error branch from d8e358b to c167f70 Compare May 5, 2023 18:47
@yihuaf yihuaf force-pushed the yihuaf/more-error branch from c167f70 to fdaa26c Compare May 5, 2023 18:49
@utam0k
Copy link
Member

utam0k commented May 6, 2023

@squili
Could you review this PR?

Copy link
Contributor

@squili squili left a comment

Choose a reason for hiding this comment

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

Looks great - just a minor nitpick

crates/libcontainer/src/notify_socket.rs Outdated Show resolved Hide resolved
Signed-off-by: yihuaf <[email protected]>
@yihuaf yihuaf merged commit 5c31fae into youki-dev:main May 8, 2023
@yihuaf yihuaf deleted the yihuaf/more-error branch May 8, 2023 03:24
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