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

Fixes undefined X and Y of onNearestXY #1000

Merged
merged 4 commits into from Dec 31, 2018

Conversation

@Domino987
Copy link
Contributor

Domino987 commented Oct 9, 2018

Fixes a bug, where onNearestXY returns undefined for innerX and innerY, since VoronoiSite<[number, number]> ( the return of voronoiInstance.find() ) has no x and y but an array of numbers.

Fixes a bug, where onNearestXY returns undefined for innerX and innerY, since VoronoiSite<[number, number]> ( the return of voronoiInstance.find() ) has no x and y but an array of numbers.
@mcnuttandrew

This comment has been minimized.

Copy link
Collaborator

mcnuttandrew commented Oct 9, 2018

Hey @Domino987

Thanks for catching that! Will you write a test documenting the bug/fix so that we don't back slide in the future?

@mklanjsek

This comment has been minimized.

Copy link

mklanjsek commented Dec 26, 2018

@mcnuttandrew @Domino987 any updates on this? We are running into same issue and would love to see it fixed.

@Domino987

This comment has been minimized.

Copy link
Contributor Author

Domino987 commented Dec 30, 2018

Hi sorry for the delay. I added a test to evaluate the results of on nearest xy. Please review @mcnuttandrew .

Copy link
Collaborator

mcnuttandrew left a comment

Thanks for doing this!

@mcnuttandrew

This comment has been minimized.

Copy link
Collaborator

mcnuttandrew commented Dec 31, 2018

Looks like you are getting a lint error, try cleaning that up and integration will pass.

Domino987 added 2 commits Dec 31, 2018
@Domino987

This comment has been minimized.

Copy link
Contributor Author

Domino987 commented Dec 31, 2018

Ok fixed it. @mcnuttandrew

@mcnuttandrew

This comment has been minimized.

Copy link
Collaborator

mcnuttandrew commented Dec 31, 2018

Wonderful! Looks great.

@mcnuttandrew mcnuttandrew merged commit 70e4f6f into uber:master Dec 31, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@mklanjsek

This comment has been minimized.

Copy link

mklanjsek commented Jan 8, 2019

Thanks for completing that, can't wait to try it in next release!

@mcnuttandrew

This comment has been minimized.

Copy link
Collaborator

mcnuttandrew commented Jan 8, 2019

@benshope mind cutting a patch?

@benshope

This comment has been minimized.

Copy link
Contributor

benshope commented Jan 14, 2019

@mcnuttandrew Not at all! Sorry for the delay - pushed 1.11.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.