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

Migrate ArrayTools to functions #15385

Merged
merged 2 commits into from
Aug 8, 2024
Merged

Conversation

james-pre
Copy link
Contributor

This PR migrates ArrayTools from a class with static methods and properties into a set of export functions.

  • Backward compatible
  • Backward-compatible exports marked as deprecated
  • PascalCase is followed for exported functions' names

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 8, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 8, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 8, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Aug 8, 2024

@RaananW
Copy link
Member

RaananW commented Aug 8, 2024

Same thing regarding export from index. And let's delay further changes to these classes. Whereas I agree that a class with only static members is simply an object, there are some minor differences that we need to consider (for example, docs are suddenly different).

@james-pre
Copy link
Contributor Author

Same thing regarding export from index.

Funny enough, ArrayTools isn't exported. This means it doesn't even appear in docs.

And let's delay further changes to these classes. Whereas I agree that a class with only static members is simply an object, there are some minor differences that we need to consider (for example, docs are suddenly different).

Sounds good, we can talk about this more on the forums.

@RaananW
Copy link
Member

RaananW commented Aug 8, 2024

Funny enough, ArrayTools isn't exported. This means it doesn't even appear in docs.

If it is internal there is no need to export it, and there is no need to create an object for it. Can simply be converted to functions

@james-pre
Copy link
Contributor Author

james-pre commented Aug 8, 2024

If it is internal there is no need to export it, and there is no need to create an object for it. Can simply be converted to functions

Got it, I removed the ArrayTools object since it isn't exported and has no more internal references.

@RaananW RaananW merged commit db5c8b6 into BabylonJS:master Aug 8, 2024
10 of 12 checks passed
@james-pre james-pre deleted the fix-arraytools branch August 8, 2024 16:54
@james-pre james-pre changed the title Migrate ArrayTools from a static class to functions Migrate ArrayTools to functions Aug 9, 2024
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