pesos / grofer

A system and resource monitoring tool written in Golang!

Apache License 2.0

352 stars 52 forks source link

Remove `mpstat` as a dependency for the CPU Info page #116

Closed panzerox123 closed 3 years ago

panzerox123 commented 3 years ago

Description

Fixes #97

Read /proc/stat to obtain CPU load information instead of relying on mpstat. No more dependency related errors on systems that don't have mpstat installed by default.
Updated CONTRIBUTING.MD to say main instead of master branch.

Let me know if there are any changes you would like :D

Type of change

Please delete options that are not relevant.

[X] Bug fix (non-breaking change which fixes an issue)

Checklist:

[X] I have read the contribution guidelines and followed it as far as possible.
[X] I have performed a self-review of my own code (if applicable)
[X] I have commented my code, particularly in hard-to-understand areas (if applicable)
[X] I have run go fmt on my code (reference)
[ ] I have made corresponding changes to the documentation (if applicable)
[X] My changes generate no new warnings
[ ] Any dependent and pending changes have been merged and published

Gituser143 commented 3 years ago

@panzerox123 Well that was fast xD

panzerox123 commented 3 years ago

@panzerox123 Well that was fast xD

Yeah lol I only asked to be assigned once I figured it out xD

panzerox123 commented 3 years ago

Thanks for the reviews guys! I'll make the necessary changes and push another commit tomorrow!

panzerox123 commented 3 years ago

Would it be okay if I just passed a pointer to c from UpdateCPULoad, instead of creating a new type which will mostly consist of the same data?

Gituser143 commented 3 years ago

Would it be okay if I just passed a pointer to c from UpdateCPULoad, instead of creating a new type which will mostly consist of the same data?

Yep this is good!

MadhavJivrajani commented 3 years ago

A few more suggestions coming your way @panzerox123!

Gituser143 commented 3 years ago

@panzerox123 Before along with your next commit, do run go mod tidy once. It should remove the unnecessary dependencies and I'm hoping it fixes the failing checks at the go get step :P

panzerox123 commented 3 years ago

@panzerox123 Before along with your next commit, do run go mod tidy once. It should remove the unnecessary dependencies and I'm hoping it fixes the failing checks at the go get step :P

Cool! And should I include the go.mod and go.sum files as well in the next commit?

Gituser143 commented 3 years ago

Cool! And should I include the go.mod and go.sum files as well in the next commit?

Yep! They aren't git ignored or anything, so make sure to push them.

MadhavJivrajani commented 3 years ago

Also if you want to play around with the benchmark:

# run the benchmark
go test -bench=. -benchtime=100000x -benchmem

panzerox123 commented 3 years ago

Yeah let me try these out!

panzerox123 commented 3 years ago

Looks like the go get error seems to be fixed as well

MadhavJivrajani commented 3 years ago

Will merge after addition of comments.

Gituser143 commented 3 years ago

@panzerox123 @MadhavJivrajani sorry for the weird branch updates. Since CI builds weren't triggering particularly for this PR (for some unknown/weird reason), I tried setting them up through actions. :P

MadhavJivrajani commented 3 years ago

@Gituser143 build is stuck again 🤣

MadhavJivrajani commented 3 years ago

I'm gonna merge this considering the previous commit passed all tests and the build was successful and this commit is just addition of comments.