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/Buffer - More precise documentation of supported units for buffering #2218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcjansen
Copy link

@marcjansen marcjansen commented Oct 31, 2021

This changes the documentation of the buffer module to list all supported units.

lengthToUnits (which is used when buffering) supports more than just miles, kilometers, and degrees. I am not sure if this was not documented on purpose.

I also wasn't sure whether I was supposed to commit the changed README.md as well.

Please review.


Please fill in this template.

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

Submitting a new TurfJS Module.

Not applicable

@@ -11,7 +11,18 @@ import {
} from "@turf/helpers";

/**
* Calculates a buffer for input features for a given radius. Units supported are miles, kilometers, and degrees.
* Calculates a buffer for input features for a given radius. Units supported are:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think that this is still a useful thing to add to the docs? I'd be inclined to just point users towards the typescript autocomplete for this field. Otherwise I'm mostly just concerned about keeping this list up to date with the actual definition in @turf/helpers. Maybe that's a silly concern since we probably don't often get new units of length.

Copy link
Member

Choose a reason for hiding this comment

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

We should definitely try to avoid repeatedly listing all the options in the individual packages.

There might have been autocomplete issues in the past due to .js/.dts/JSDoc inconsistencies. For example, some places have Units in the function definition, but then just string in the JSDoc. Hopefully as we weed out the last few JS packages that will self resolve.

For this case maybe we say, leave any mention of units off completely. What do you think @marcjansen and @mfedderly?

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.

3 participants