Skip to content

[three] BufferGeometry.setFromPoints accepts Vector2s as well - #25897

Merged
DanielRosenwasser merged 1 commit into
DefinitelyTyped:masterfrom
fordhurley:three-buffer-geom-points
May 23, 2018
Merged

[three] BufferGeometry.setFromPoints accepts Vector2s as well#25897
DanielRosenwasser merged 1 commit into
DefinitelyTyped:masterfrom
fordhurley:three-buffer-geom-points

Conversation

@fordhurley

Copy link
Copy Markdown
Contributor

The documentation shows examples of this, e.g.:

https://threejs.org/docs/#api/extras/curves/EllipseCurve

The source code shows how it handles an array of either type:

https://github.com/mrdoob/three.js/blob/2f2e5f731/src/core/BufferGeometry.js#L352

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

@typescript-bot

typescript-bot commented May 20, 2018

Copy link
Copy Markdown
Contributor

@fordhurley Thank you for submitting this PR!

🔔 @gyohk @florentpoujol @SereznoKot @omni360 @ivoisbelongtous @piranha771 @qszhusightp @nakakura @s093294 @Pro @efokschaner @PsychoSTS @dhritzkiv @apurvaojas - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@efokschaner efokschaner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the improvement.
I approve of the change as is.

Potentially an array of anything with x and y and z? members would suffice, but I always wonder whether three expects these to be vector classed objects even if it doesn't depend on that right now...

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels May 20, 2018
@typescript-bot

Copy link
Copy Markdown
Contributor

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@DanielRosenwasser
DanielRosenwasser merged commit b062863 into DefinitelyTyped:master May 23, 2018
@DanielRosenwasser

Copy link
Copy Markdown
Member

Thanks all!

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

Labels

Owner Approved A listed owner of this package signed off on the pull request.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants