Skip to content

lineToPolygon fix. Resolves #1812#1879

Merged
mfedderly merged 6 commits intoTurfjs:masterfrom
MortenBirk:lineToPolygonInputMutation
May 4, 2020
Merged

lineToPolygon fix. Resolves #1812#1879
mfedderly merged 6 commits intoTurfjs:masterfrom
MortenBirk:lineToPolygonInputMutation

Conversation

@MortenBirk
Copy link
Copy Markdown
Contributor

Ensure that the input is by default not modified when lineToPolygon is called. Resolves #1812

An optional mutate prop has been added to options. If true the input will be modified, if false (default) the input will not be modified.

  • 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.

…s called.

An optional mutate prop has been added to options. If true the input will be modified, if false (default) the input will not be modified.
Copy link
Copy Markdown
Collaborator

@mfedderly mfedderly left a comment

Choose a reason for hiding this comment

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

@rowanwins This is potentially an API break because the default behavior is changing. Do you want to ship this as a bugfix anyhow?

Comment thread packages/turf-line-to-polygon/index.ts Outdated
@rowanwins
Copy link
Copy Markdown
Member

I think this is fine as a bugfix even though the default behaviour is changing.

@rowanwins rowanwins self-requested a review May 2, 2020 10:40
Copy link
Copy Markdown
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.

Looks good to me. Ensuring inputs aren't modified, or at least making people flag that they are comfortable with inputs being modified, is the preferred behaviour.

@mfedderly mfedderly merged commit 26d213d into Turfjs:master May 4, 2020
@mfedderly
Copy link
Copy Markdown
Collaborator

Thanks!

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.

lineToPolygon(line) mutates the underlying LineString

3 participants