Add CpuTimes and CpuPercent#28
Conversation
|
Any update on this issue ? Can I do something to help this PR to get merged ? |
|
I'll try and find some time to review this soon - there are a lot of changes here! (In the future, it's probably best to make formatting changes to existing code in a separate PR, so that it's easy for reviewers to see what has been added.) |
borntyping
left a comment
borntyping
left a comment
There was a problem hiding this comment.
I think this looks good. I've added a few comments in places where some minor cleanup is needed (mostly new() functions and repeated code).
@rkday, did you want to take a look at this too?
src/system.rs
Outdated
|
|
||
| impl CpuTimes { | ||
| /// Initialize CpuTimes struct | ||
| pub fn new( |
There was a problem hiding this comment.
I'm not sure this needs a new() method on the structs if the method does no additional processing. (Though I'm not very up to date on current standards.)
There was a problem hiding this comment.
I added it by coherence with the existing code but I didn't check that the existing new() methods had additional processing.
src/system.rs
Outdated
| if self.total_time() > past_cpu_times.total_time() { | ||
| let diff_total = (self.total_time() - past_cpu_times.total_time()) as f64; | ||
| CpuTimesPercent::new( | ||
| { |
There was a problem hiding this comment.
The blocks here seem to repeat multiple times, it may be worth using a function here to avoid repetition and make it easier to read.
src/system.rs
Outdated
| if fields.len() < 10 { | ||
| return Err(Error::new( | ||
| ErrorKind::InvalidData, | ||
| format!("Wrong format of /proc/stat line : {}, Maybe the kernel version is too older (Linux 2.6.33)", cpu_line), |
There was a problem hiding this comment.
Should be maybe the kernel version is too old.
src/system.rs
Outdated
| /// | ||
| /// Use informations contains in '/proc/stat' | ||
| pub fn cpu_times_percent(interval: f64) -> Result<CpuTimesPercent> { | ||
| if interval <= 0. { |
There was a problem hiding this comment.
This block (line 908 to 918) gets used again in cpu_times_percent_percpu and should probably be moved into a private function.
|
Looks good - I merged #31 first and it's left some conflicts that need resolving but happy to merge after that. |
|
Thanks! I just resolved the conflicts. |
|
I'll try and look at the others soon - may be worth checking them for merge conflicts. |
2 participants
Adding some cpu features from psutil.