rust-vmm / seccompiler

Provides easy-to-use Linux seccomp-bpf jailing.
https://crates.io/crates/seccompiler
Apache License 2.0
70 stars 10 forks source link

Implement From<BackendError> for Error #38

Closed ramyak-mehra closed 2 years ago

ramyak-mehra commented 2 years ago

Signed-off-by: Ramyak Mehra rmehra_be19@thapar.edu

Summary of the PR

Implements From for BackednError and JsonFrontentError for Erro in lib.rs and replaced map_err wherever neccessary

Requirements

Before submitting your PR, please make sure you addressed the following requirements:

ramyak-mehra commented 2 years ago

Looks good, thanks for the contribution. In order to have the tests pass, either lower the coverage to the reported value or add some tests that perform the simple casts. That's fine by me either way, since the added code simple enough.

I don't see a need to write test for these. I just wanted to know are there any other place should I make similar changes, or add these changes for any other error?

alindima commented 2 years ago

Looks good, thanks for the contribution. In order to have the tests pass, either lower the coverage to the reported value or add some tests that perform the simple casts. That's fine by me either way, since the added code simple enough.

I don't see a need to write test for these. I just wanted to know are there any other place should I make similar changes, or add these changes for any other error?

I don't see another place that needs changes. @andreeaflorescu @petreeftime WDYT? The other changes should be made by applications consuming seccompiler (but not neccessarily, as it's not a breaking change). In that case, you may just update the coverage in coverage_config_x86_64.json with the new value.

alindima commented 2 years ago

Please make sure to add a Signed-off-by line to the second commit

andreeaflorescu commented 2 years ago

I would suggest to also add Fixes: #38 to the commit message so that the issue is automatically closed when the PR is merged. This is optional, and you can do it for your future PRs.