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.

Sign up for GitHub

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

Jump to bottom

Gauge limits #5156

Merged
merged 11 commits into from
May 20, 2022
Merged

Gauge limits #5156

merged 11 commits into from
May 20, 2022

Conversation

nikhilmandlik
Copy link
Contributor

@nikhilmandlik nikhilmandlik commented May 3, 2022 •
edited

Closes #5155 #5139

Describe your changes:

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Unit tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue?

Reviewer Checklist

  • Changes appear to address issue?
  • Changes appear not to be breaking changes?
  • Appropriate unit tests included?
  • Code style and in-line documentation are appropriate?
  • Commit messages meet standards?
  • Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
  • Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

Screen Shot 2022-05-03 at 9 57 04 AM

@nikhilmandlik nikhilmandlik changed the base branch from master to release/2.0.3 May 3, 2022 01:09
@codecov
Copy link

codecov bot commented May 3, 2022 •
edited

Codecov Report

Merging #5156 (3594ec0) into master (037886a) will decrease coverage by 0.08%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5156      +/-   ##
==========================================
- Coverage   50.02%   49.94%   -0.09%     
==========================================
  Files         548      548              
  Lines       20108    20128      +20     
  Branches     1859     1863       +4     
==========================================
- Hits        10060    10053       -7     
- Misses       9570     9593      +23     
- Partials      478      482       +4
Impacted Files Coverage Δ
src/plugins/gauge/components/Gauge.vue 51.72% <20.00%> (-7.48%) ⬇️
src/plugins/gauge/gauge-limit-util.js 41.66% <0.00%> (-50.00%) ⬇️
...ettingsSynchronizer/URLTimeSettingsSynchronizer.js 96.96% <0.00%> (-1.02%) ⬇️
...c/plugins/persistence/couch/CouchObjectProvider.js 81.77% <0.00%> (-0.45%) ⬇️

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 037886a...3594ec0. Read the comment docs.

@unlikelyzero unlikelyzero self-requested a review May 3, 2022 17:22
unlikelyzero
unlikelyzero requested changes May 3, 2022
View reviewed changes
Copy link
Collaborator

@unlikelyzero unlikelyzero left a comment

Choose a reason for hiding this comment

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

@nikhilmandlik please review the visual test change and also modify the target branch to master

@nikhilmandlik nikhilmandlik changed the base branch from release/2.0.3 to master May 3, 2022 17:45
shefalijoshi and others added 4 commits May 3, 2022 11:12
@shefalijoshi @nikhilmandlik
Fix tick values for plots ticks in log mode and null check (#5119)
0e6be1b 
* [2297] When there is no display range or range, skip setting the range value when auto scale is turned off.

* If the formatted value is a number and a float, set precision to 2 decimal points.

* Fix value assignment

* Use whole numbers in log mode

* Revert whole numbers fix - need floats for values between 0 and 1.
@akhenry @nikhilmandlik
Reverts forced precision for log plots axis labels (#5147)
cfb000d
@nikhilmandlik
[Gauge Plugin] Limits and Composition Issues #5155
56bd399
@nikhilmandlik
default current value as '--'
d3b1cc2
@nikhilmandlik nikhilmandlik mentioned this pull request May 3, 2022
Closed
15 tasks
@unlikelyzero unlikelyzero self-requested a review May 3, 2022 20:57
@nikhilmandlik nikhilmandlik linked an issue May 3, 2022 that may be closed by this pull request
Unset Gauge limits are being handled as 0 #5139
Closed
5 tasks
@akhenry akhenry requested review from jvigliotta and removed request for unlikelyzero May 9, 2022 21:25
jvigliotta
jvigliotta requested changes May 17, 2022
View reviewed changes
Copy link
Contributor

@jvigliotta jvigliotta left a comment

Choose a reason for hiding this comment

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

Looking nice! Can we change the v-if's to check computed values instead of containing logic? That should give a little performance bump due to caching.

charlesh88
charlesh88 approved these changes May 17, 2022
View reviewed changes
Copy link
Contributor

@charlesh88 charlesh88 left a comment

Choose a reason for hiding this comment

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

LGTM

jvigliotta
jvigliotta approved these changes May 20, 2022
View reviewed changes
Copy link
Contributor

@jvigliotta jvigliotta left a comment

Choose a reason for hiding this comment

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

Nice! LGTM!

@jvigliotta jvigliotta enabled auto-merge (squash) May 20, 2022 20:23
@nikhilmandlik
Copy link
Contributor Author

Nice! LGTM!

ship it

@jvigliotta jvigliotta merged commit 9f9c69e into master May 20, 2022
@nikhilmandlik nikhilmandlik deleted the gauge-limits branch May 20, 2022 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlesh88 charlesh88 charlesh88 approved these changes

@jvigliotta jvigliotta jvigliotta approved these changes

@unlikelyzero unlikelyzero unlikelyzero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Gauge Plugin] Limits and Composition Issues Unset Gauge limits are being handled as 0
6 participants
@nikhilmandlik @charlesh88 @jvigliotta @unlikelyzero @shefalijoshi @akhenry