gui icon indicating copy to clipboard operation
gui copied to clipboard
verified publisher icon bitcoin-core

Network graph improvements

Open rebroad opened this issue 3 years ago • 5 comments

This combines #492, #483, #484, and #473 and also fixes #532 by adding the ability to change between graph durations without losing the data (it's sampled at each interval). Also added is some graphical animations when the X or Y scale changes. (it might be necessary to make this optional on lower performing hardware, such as the raspberry pi).

One more feature is that the graph will start on the smallest duration, and automatically increase the graph duration once the graph is filled.

Some examples can be seen in my comment at https://github.com/bitcoin-core/gui/pull/539#issuecomment-1031852315

I'm expecting there may be issues with my coding style, so I'll create this PR as a draft at this stage.

rebroad avatar Feb 24 '22 21:02 rebroad

What's the rationale for combining those PRs in this one?

On Thu, Feb 24, 2022 at 10:11 PM Rebroad @.***> wrote:

This combines #492 https://github.com/bitcoin-core/gui/pull/492, #483 https://github.com/bitcoin-core/gui/pull/483, #484 https://github.com/bitcoin-core/gui/pull/484, and #473 https://github.com/bitcoin-core/gui/pull/473 and also adds the ability to change between graph durations without losing the data (it's sampled at each interval). Also added is some graphical animations when the X or Y scale changes. (it might be necessary to make this optional on lower performing hardware, such as the raspberry pi).

Some examples can be seen in my comment at #539 (comment) https://github.com/bitcoin-core/gui/pull/539#issuecomment-1031852315

I'm expecting there may be issues with my coding style, so I'll create this PR as a draft at this stage.

You can view, comment on, or merge this pull request online at:

https://github.com/bitcoin-core/gui/pull/559 Commit Summary

File Changes

(9 files https://github.com/bitcoin-core/gui/pull/559/files)

Patch Links:

  • https://github.com/bitcoin-core/gui/pull/559.patch
  • https://github.com/bitcoin-core/gui/pull/559.diff

— Reply to this email directly, view it on GitHub https://github.com/bitcoin-core/gui/pull/559, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4W3BURANHIF5WEZW3NLU42NIFANCNFSM5PIP6G4Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

katesalazar avatar Feb 24 '22 21:02 katesalazar

What's the rationale for combining those PRs in this one?

Because they are difficult to separate - given they each kinda tread on the toes of the others, I thought better to PR the whole foot rather than the individual toes. e.g. more than one feature uses the y_value() function, the tool-tip change and the linear/non-linear change both need an extra commit to be able work together.

rebroad avatar Feb 24 '22 21:02 rebroad

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #540 (network graph - show/hide panels based on window width/height by RandyMcMillan)
  • #483 (Make network graph slider easier to use by rebroad)
  • #473 (Enable a non-linear network traffic option (click to toggle between linear and non-linear) by rebroad)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot avatar Feb 25 '22 05:02 DrahtBot

This was a bad idea. If there wasn't enough developer time to review your scattered PRs, a larger combined PR shall only have larger reviewing problems. I'd suggest choosing one of your small PRs and to pay for reviews. Get it definitely NACKed or ACKed and merged, then repeat with a different PR. Choose PR order wisely. Good luck!

On Thu, Feb 24, 2022 at 10:34 PM Rebroad @.***> wrote:

What's the rationale for combining those PRs in this one?

Because they are difficult to separate - given they each kinda tread on the toes of the others, I thought better to PR the whole foot rather than the individual toes. e.g. more than one feature uses the y_value() function, the tool-tip change and the linear/non-linear change both need an extra commit to be able work together.

— Reply to this email directly, view it on GitHub https://github.com/bitcoin-core/gui/pull/559#issuecomment-1050287182, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMRS4W3MP43PNFYLTSFJKMDU42P55ANCNFSM5PIP6G4Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

katesalazar avatar Sep 18 '22 19:09 katesalazar

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

DrahtBot avatar Oct 03 '22 12:10 DrahtBot