regolith-i3xrocks-config
regolith-i3xrocks-config copied to clipboard
regolith-linux
Add support for multiple network interface stats
Some of my machines have multiple default interfaces and this caused the net-traffic script to not work. This pull request adds support for multiple interfaces and is backward compatible with the original script.
Hi @aacero thanks for your contribution! The code looks very clean. I ran it locally and didn't find any issues, although I don't have multiple active network interfaces.
It is common in this part of Regolith (status indicators) that a seemingly benign change causes regressions for users with various configurations we didn't consider. Unfortunately due to the nature of i3blocks these errors can result in big problems like spamming syslog or consuming 100% of a cpu core. Unrelated to your PR, but generally I have low confidence that any indicator shell script changes won't result in some regressions for some users.
Given this I propose that in the short term we rename this such that the existing script and your updates are available independently for users. Those wishing for network status for multiple interfaces would install another package, say i3xrocks-net-traffic-multi-nic or something. It would follow that rather than modification to net-traffic this would be a new script, net-traffic-multi-nic (or whatever is the better name).
Great, can you update the PR such that your changes are in new file as described? Then I'll merge and add a i3xrocks config script and package it for testing.
Will do. I currently don't have a way to test so it might be a week or two before I resubmit a PR
On Sun, Jul 4, 2021, 11:20 Ken Gilmer @.***> wrote:
If you rebase from master the test regression should disappear @aacero https://github.com/aacero
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/regolith-linux/regolith-i3xrocks-config/pull/119#issuecomment-873610638, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFSDKETQ5ELHL3JSH3IUQDTWB34TANCNFSM426VZ2DQ .