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

part of PR 340 moving syscalls into structure #357

Conversation

tommady
Copy link
Collaborator

@tommady tommady commented Oct 4, 2021

as requested by separating #340 into small PRs

this one is the second part of moving all system calls into a RootFS structure to make them mockable.
please help review 🙇🏻

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2021

Codecov Report

Merging #357 (e76a9d1) into main (e0d9cea) will decrease coverage by 0.27%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #357      +/-   ##
==========================================
- Coverage   68.83%   68.55%   -0.28%     
==========================================
  Files          48       48              
  Lines        7181     7210      +29     
==========================================
  Hits         4943     4943              
- Misses       2238     2267      +29     

src/rootfs.rs Outdated
Some(uknown) => bail!("unknown rootfs_propagation: {}", uknown),
/// Holds information about rootfs
pub struct RootFS {
command: Box<dyn Syscall>,
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this s mixed up by my mistake, but let's unify the name to syscall.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean the field name from command to syscall?
or something else, please guide.

Copy link
Member

Choose a reason for hiding this comment

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

Let's simply change the name command to syscall.

@utam0k
Copy link
Member

utam0k commented Oct 4, 2021

@tommady This PR just uses Syscalls so that mocks can be used, right?

@tommady
Copy link
Collaborator Author

tommady commented Oct 4, 2021

@tommady This PR just uses Syscalls so that mocks can be used, right?

yes correctly!

@utam0k
Copy link
Member

utam0k commented Oct 4, 2021

@tommady Thanks for always separating the PR from the rest. It makes it easier for me to review them :)

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

LGTM

@utam0k utam0k merged commit 8bc8300 into youki-dev:main Oct 4, 2021
@tommady tommady deleted the 279-increate-the-code-coverage-of-src-rootfs-part-2 branch October 4, 2021 01:41
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.

3 participants