Building quark #10
Building quark #10
Conversation
Signed-off-by: JeremyLARDENOIS <[email protected]>
sameo
left a comment
sameo
left a comment
There was a problem hiding this comment.
Thanks for the PR @JeremyLARDENOIS . I have a few comments on the code and an additional one on the PR/commit: Your single commit should explain what it's doing and why it's doing it. Building quarkwont allow anyone going through future git logs to understand what this commit is about.
|
|
||
| if !self.offline { | ||
| println!("Online mode is not supported yet."); | ||
| return Ok(()); |
There was a problem hiding this comment.
If it's not supported, you should return an error.
| .arg(src) | ||
| .arg(workdir) | ||
| .status() | ||
| .unwrap(); |
There was a problem hiding this comment.
No unwrap, please propagate the error instead.
Also, you should consider using the https://docs.rs/fs_extra/1.2.0/fs_extra/index.html crate. It's only based on the std crate.
There was a problem hiding this comment.
I wanted to use it, but I didn't achieve it. I will try again I think
| } else { | ||
| // If the rootfs doesn't exist, display error message | ||
| println!("Rootfs doesn't exist."); | ||
| return Ok(()); |
|
|
||
| // Change the init script to use the kaps bundle | ||
| println!("Changing init script to use the kaps bundle"); | ||
| let init_script = format!("{}/{}/init", workdir, rootfs_filename); |
There was a problem hiding this comment.
A cleaner way to build an actual path is to use the std::path::Pathcrate.
| let mut init_script_file = std::fs::File::create(init_script.clone())?; | ||
| println!("Writing new init script"); | ||
| let kaps_name = self.kaps.split('/').last().unwrap(); | ||
| let bundle_name = self.bundle.split('/').last().unwrap(); |
There was a problem hiding this comment.
Lots of unwraps, lots of potential panics. Return an error (through e.g.map_err).
| // Create initramfs ------------------------------------------------------------ | ||
| println!("Creating initramfs"); | ||
| let rootfs_workdir = format!("{}/{}", workdir, rootfs_filename); | ||
| (subprocess::Exec::shell(format!("find {} -print0", rootfs_workdir)) |
There was a problem hiding this comment.
Because there is a bash pipe, and I don't know how to do it without make a bash script
Signed-off-by: JeremyLARDENOIS <[email protected]>
2 participants
Signed-off-by: JeremyLARDENOIS [email protected]
@sameo Can I have a review? 🙂