search

Njanderson / resmon

Displays resource utilization in the VSCode status bar.
81 stars 22 forks source link

Typos, vulnerability fix, & features #23 & #33 #35

Open DariusHutchison opened 4 years ago

Njanderson commented 4 years ago

Thanks so much for working on this! I'll take a look this weekend.

DariusHutchison commented 4 years ago

No problem. It was my first time working with typescript. Hope it isn't too bad.

DariusHutchison commented 4 years ago

Any update on the review?

Njanderson commented 4 years ago

Hey there!

See my latest commit with the new function getPrecision() for #23 , and then look at the _maxWidth. There's some discussion in the issue #33 that I added about the struggles of having a monospace font. I think these are what these features were about.

For #23 , I think the ask is to change the precision from 9.54% to 9.5% or 10% (rounded up), whereas I think your change gives a leading 0 when you have <10%, so you'd have 09.54%, when I think they wanted the float part to be rounded.

I think I do need to add many of the documentation and some of the formatting things you added, though, still looking. Thanks, Nick

DariusHutchison commented 3 years ago

Hey there!

See my latest commit with the new function getPrecision() for #23 , and then look at the _maxWidth. There's some discussion in the issue #33 that I added about the struggles of having a monospace font. I think these are what these features were about.

For #23 , I think the ask is to change the precision from 9.54% to 9.5% or 10% (rounded up), whereas I think your change gives a leading 0 when you have <10%, so you'd have 09.54%, when I think they wanted the float part to be rounded.

I think I do need to add many of the documentation and some of the formatting things you added, though, still looking. Thanks, Nick

Fair enough feel free to change how you need to and close if you think its better without merging, especially if a monospaced font fixes the issue with padding.