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

Remove typeInfo struct from lmms_basics.h #7380

Merged
merged 14 commits into from
Aug 7, 2024

Conversation

Rossmaxx
Copy link
Contributor

The typeInfo struct mostly consists of functions which are already a part of the standard math library, and for the other 2 functions, they should belong in lmms_math.h

include/lmms_math.h Outdated Show resolved Hide resolved
include/lmms_math.h Outdated Show resolved Hide resolved
@Rossmaxx Rossmaxx requested a review from sakertooth July 16, 2024 04:47
@DomClark DomClark self-requested a review July 16, 2024 07:59
@Rossmaxx Rossmaxx mentioned this pull request Jul 19, 2024
include/lmms_math.h Outdated Show resolved Hide resolved
include/lmms_math.h Outdated Show resolved Hide resolved
Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

I think this is fine for now, though there might be other thoughts. Apologies for thinking the exponential was subtraction and effectively suggesting how to break the build (might be getting a bit rusty, I should review the language again).

@Rossmaxx
Copy link
Contributor Author

Apologies for thinking the exponential was subtraction

I too thought the same, and asked gpt. It said it's exponential.

include/lmms_math.h Outdated Show resolved Hide resolved
@Rossmaxx
Copy link
Contributor Author

This PR is now ready.

@Rossmaxx
Copy link
Contributor Author

Rossmaxx commented Aug 4, 2024

I'll merge this in 2 days if no one objects.

@Rossmaxx Rossmaxx merged commit 828cefb into LMMS:master Aug 7, 2024
10 checks passed
@Rossmaxx Rossmaxx deleted the remove-typeinfo branch August 7, 2024 02:03
sakertooth pushed a commit that referenced this pull request Aug 7, 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.

None yet

4 participants