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

fix(stats): passing value 0 to valueScale #993

Merged
merged 2 commits into from
Jan 6, 2021
Merged

fix(stats): passing value 0 to valueScale #993

merged 2 commits into from
Jan 6, 2021

Conversation

rsbh
Copy link
Contributor

@rsbh rsbh commented Jan 6, 2021

🐛 Bug Fix

  • right now there is a check for the values before calling valueScale which is supposed to check undefined and null value but it is filtering out the value 0
    when an inverted linear scale is passed as valueScale the value 0 is at the top, not the bottom. And values can be 0 and it is a valid value for valueScale function.

@coveralls
Copy link

coveralls commented Jan 6, 2021

Pull Request Test Coverage Report for Build 465621721

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 61.196%

Totals Coverage Status
Change from base Build 462020185: 0.02%
Covered Lines: 1728
Relevant Lines: 2690

💛 - Coveralls

@rsbh rsbh changed the title fix(stats): passing min value 0 to valueScale fix(stats): passing value 0 to valueScale Jan 6, 2021
Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @rsbh! 🙏 I agree this is the desired behavior 👍

@williaster williaster added this to the 1.4.0 milestone Jan 6, 2021
@williaster williaster merged commit 3032ded into airbnb:master Jan 6, 2021
@rsbh
Copy link
Contributor Author

rsbh commented Jan 7, 2021

@williaster what is the ETA on the 1.4.0 release?

@williaster
Copy link
Collaborator

@rsbh I was trying to get a few more PRs in this week before release but they'll probably land early next week. If you need the release urgently I can probably get a release out today with this fix.

@rsbh
Copy link
Contributor Author

rsbh commented Jan 9, 2021

Thanks @williaster. This is a bug fix and it will be better to release a patch version instead of waiting for the next major/minor release.

@williaster
Copy link
Collaborator

👍 yep, this was a bug fix which would normally be a patch, but there were already new features merged by the time this got in so went ahead with the minor release. 1.4.0 is out now 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants