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

⬆️ [@turf/intersect][@turf/difference][@turf/union] bump polygon-clipping to 0.15.3 #2085

Merged
merged 5 commits into from
Jun 20, 2021

Conversation

twelch
Copy link
Collaborator

@twelch twelch commented May 26, 2021

This PR bumps turf packages to the latest polygon-clipping 0.15.3, which upgraded to martinez-polygon-clipping 0.7.0 (mfogel/polygon-clipping#114) and has valuable improvements for features with holes (w8r/martinez#113).

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Have read How To Contribute.
  • Run npm test at the sub modules where changes have occurred.
  • Run npm run lint to ensure code style at the turf module level.

@rowanwins
Copy link
Member

@twelch Thanks for the contribution
Although unfortunately this PR wont actually fix anything to do with holes as the martinez PR you referred to is only a devDependency for the polygon-clipping library that is used for benchmarks and comparing outputs.

@twelch
Copy link
Collaborator Author

twelch commented Jun 7, 2021

Fair enough, you were involved with that PR after all 😄 Martinez 0.7 still has improvements that I require for my work, and maintain a fork of turf-intersect to get. Let me know if anything more is needed to approve.

@twelch
Copy link
Collaborator Author

twelch commented Jun 13, 2021

I see that the yarn lockfile changes that crept into this small PR are fairly substantial and possibly due to my running a newer (currently latest) yarn version 1.22.10 that changes the structure. Perhaps there is a larger overall upgrade to the lockfile needed before this PR gets merged.

@twelch
Copy link
Collaborator Author

twelch commented Jun 15, 2021

I'm just now tuning in to the work that has gone into the v7 branch - #1432.

[Edit] Okay I'm seeing other issues that relay the v7 branch is defunct. I guess I'll wait for word 😅 . fwiw, I'm interested in contributing more substantially it's just not clear how/where/who to plug in. This PR is a test in a way for me.

@twelch
Copy link
Collaborator Author

twelch commented Jun 16, 2021

I removed the lockfile changes

@rowanwins rowanwins self-requested a review June 20, 2021 11:03
Copy link
Member

@rowanwins rowanwins left a comment

Choose a reason for hiding this comment

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

👍

@rowanwins rowanwins merged commit 4d84de9 into Turfjs:master Jun 20, 2021
@rowanwins
Copy link
Member

Thanks for your contribution @twelch !

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

Successfully merging this pull request may close these issues.

2 participants