Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add netspeed parameter (#1083) #2590

Merged
merged 5 commits into from
Feb 6, 2022
Merged

Add netspeed parameter (#1083) #2590

merged 5 commits into from
Feb 6, 2022

Conversation

prateekmedia
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Other: Replace this with a description of the type of this PR

Description

Adds netspeed parameter which is basically sum of up and down speeds

Related Issues & Documents

Fixes #1083, Although this doesn't add dynamic icons of up and down

Documentation (check all applicable)

  • This PR requires changes to the Wiki documentation (describe the changes)
    netspeed parameter uses needs to be described in the network module documentation.
  • This PR requires changes to the documentation inside the git repo (please add them to the PR).
  • Does not require documentation changes

src/modules/network.cpp Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 6, 2022

Codecov Report

Merging #2590 (df7d969) into master (a33e8de) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2590      +/-   ##
==========================================
- Coverage   11.38%   11.37%   -0.01%     
==========================================
  Files         153      153              
  Lines       11131    11137       +6     
==========================================
  Hits         1267     1267              
- Misses       9864     9870       +6     
Flag Coverage Δ
unittests 11.37% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/adapters/net.hpp 0.00% <ø> (ø)
src/adapters/net.cpp 0.00% <0.00%> (ø)
src/modules/network.cpp 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a33e8de...df7d969. Read the comment docs.

Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly! Code looks good now, thanks :)

One last thing, please also mention this change in the changelog in the Added section (see https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog)

@prateekmedia
Copy link
Contributor Author

@patrick96 I am also thinking to add total used net speed so someone like me can keep track of total used data of the session. Not in this PR but in next PR of course. So what do you think?

@patrick96 patrick96 merged commit ab915fb into polybar:master Feb 6, 2022
@patrick96
Copy link
Member

Merged! Thanks a lot 😃

I am also thinking to add total used net speed so someone like me can keep track of total used data of the session. Not in this PR but in next PR of course. So what do you think?

If you have a use-case for it, sure, go for it. Just note that the number of bytes received or sent we get from getifaddrs is only a uint32 so it wraps around after 4GB and isn't really useful to show the total number of bytes in the session.

@prateekmedia prateekmedia deleted the patch-1 branch February 7, 2022 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show total of %upspeed% and %downspeed% as %netspeed% in network module
2 participants