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

Tighten up integer division checks on Quantity #39

Closed
chiphogg opened this issue Dec 12, 2022 · 1 comment · Fixed by #142
Closed

Tighten up integer division checks on Quantity #39

chiphogg opened this issue Dec 12, 2022 · 1 comment · Fixed by #142
Labels
⬇️ affects: code (implementation) Affects implementation details of the code 📁 kind: bug Something isn't working correctly, or there's a mistake 💪 effort: small
Milestone

Comments

@chiphogg
Copy link
Contributor

This test fails:

EXPECT_EQ(hertz(100), 1 / milliseconds(10));

It should fail to compile. (i.e., the goal is to expand our integer division check to include the cases where only one argument is a Quantity.)

@chiphogg
Copy link
Contributor Author

By the way, and for posterity: here's how we could get that test to pass:

EXPECT_EQ(hertz(100), inverse_as(hertz, milli(seconds)(10)));

This would automatically divide the value 10 into 1000, because hertz times milli(seconds) gives a dimensionless unit whose magnitude is 1/1000.

@chiphogg chiphogg added the 📁 kind: cleanup Making the library nicer in some way, without affecting functionality much or at all label Dec 12, 2022
@chiphogg chiphogg added the 📁 kind: bug Something isn't working correctly, or there's a mistake label Jan 10, 2023
@chiphogg chiphogg added ⬇️ affects: code (implementation) Affects implementation details of the code 💪 effort: tiny A trivial change 💪 effort: small and removed 📁 kind: cleanup Making the library nicer in some way, without affecting functionality much or at all 💪 effort: tiny A trivial change labels Mar 26, 2023
@chiphogg chiphogg added this to the 0.3.3 milestone Jun 21, 2023
chiphogg added a commit that referenced this issue Jun 27, 2023
This closes a loophole and makes the library more consistent.

Fixes #39.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⬇️ affects: code (implementation) Affects implementation details of the code 📁 kind: bug Something isn't working correctly, or there's a mistake 💪 effort: small
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant