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

Pure annotations house cleaning: Fix CatmullRomCurve3 formatting #24209

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

pschroen
Copy link
Contributor

@pschroen pschroen commented Jun 8, 2022

Related issue: #24207

@LeviPesin caught a mistake in my formatting of the last PR. See #24207 (comment). 🙏

@LeviPesin
Copy link
Contributor

Can we split this line in three separate like const px = ...; const py = ...; const pz = ...?

@pschroen
Copy link
Contributor Author

pschroen commented Jun 8, 2022

I had originally done that but then changed it to that format after comparing how other cases were handled, most of the pure annotations are in the multiple variable declarations format.

What changed my mind was the rest of that file is also written as single variable declarations, and also here:

const _va = /*@__PURE__*/ new Vector3(), // from pe to pa

Happy to change it to multiple declarations for consistency with the rest of the pure annotations though. 😅

@mrdoob mrdoob added this to the r142 milestone Jun 9, 2022
@mrdoob mrdoob merged commit 7228425 into mrdoob:dev Jun 9, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jun 9, 2022

Thanks!

@pschroen pschroen deleted the tree-shaking-pure-annotations-fix branch June 9, 2022 13:28
abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
…oob#24209)

* Pure annotations house cleaning: Fix CatmullRomCurve3 formatting

* Update CatmullRomCurve3.js

Co-authored-by: mrdoob <[email protected]>
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
…oob#24209)

* Pure annotations house cleaning: Fix CatmullRomCurve3 formatting

* Update CatmullRomCurve3.js

Co-authored-by: mrdoob <[email protected]>
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.

4 participants