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

move to Required Associated Constants #311

Closed
wants to merge 2 commits into from

Conversation

BabaBert
Copy link

@BabaBert BabaBert commented Mar 6, 2024

This merge request transitions the trait methods of associated constants to actual required constants.
It also adds MANTISSA_DIGITS, DIGITS and RADIX to the Float traits.
This would close #213

@cuviper
Copy link
Member

cuviper commented Mar 7, 2024

I'm not planning to process breaking changes anytime soon, so I'd rather leave this on the issue for now. A pull request will only bitrot over time. Thanks anyway!

@cuviper cuviper closed this Mar 7, 2024
@BabaBert
Copy link
Author

BabaBert commented Mar 7, 2024

I fully understand that.
However, is it an option to add these associated on top of the current methods? That would be possible and not break any existing functionality.

@tarcieri
Copy link
Contributor

tarcieri commented Mar 7, 2024

@BabaBert see #303 as an additive PR for traits with associated constants which was accepted

@BabaBert
Copy link
Author

BabaBert commented Mar 7, 2024

@tarcieri so I should rather create (a) new trait(s) that only contain the associates and implement that instead. That's a good point, but then the question arises how that trait should be called (FloatConst is already used) and how they should be split up. I would suggest having all the constants in one place, as they are related.

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

Successfully merging this pull request may close these issues.

Make FloatConst members be const
3 participants