-
Notifications
You must be signed in to change notification settings - Fork 334
Network graph improvements #559
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
Conversation
3d282b5 to
8c56419
Compare
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. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
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
<#559 (comment)>,
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:
***@***.***>
|
|
🐙 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". |
|
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
|
Closing this due to lack of activity. Feel free to reopen. |
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 #539 (comment)
I'm expecting there may be issues with my coding style, so I'll create this PR as a draft at this stage.