Skip to content

Conversation

@chiphogg
Copy link
Member

We also add the "common point" versions of these.

The absence of common_unit and common_point_unit was mostly just an
oversight. I recently tried to call one in Godbolt, and got surprised
when I couldn't. And it turned out that I had already written this
function anyway, except that it was hidden away inside of a test file.

As I thought about these functions more, I thought it was interesting
that the inputs are unit slots, so we could have a wide variety of
things (simple units, quantity makers, unit symbols, etc.), but the
output would always be a simple unit. Wouldn't it be nice if we could
also combine meters and feet, or m and ft, and have the output
act like its inputs? This idea became the make_common and
make_common_point utilities.

Docs included (and rendered and tested).

We also add the "common point" versions of these.

The absence of `common_unit` and `common_point_unit` was mostly just an
oversight.  I recently tried to call one in Godbolt, and got surprised
when I couldn't.  And it turned out that I had already written this
function anyway, except that it was hidden away inside of a test file.

As I thought about these functions more, I thought it was interesting
that the inputs are unit slots, so we could have a wide variety of
things (simple units, quantity makers, unit symbols, etc.), but the
output would always be a simple unit.  Wouldn't it be nice if we could
also combine `meters` and `feet`, or `m` and `ft`, and have the output
act like its inputs?  This idea became the `make_common` and
`make_common_point` utilities.

Docs included (and rendered and tested).
@chiphogg chiphogg added release notes: ✨ lib (enhancement) PR enhancing the library code release notes: 📝 documentation PR affecting library documentation labels Dec 18, 2024
@chiphogg chiphogg marked this pull request as ready for review December 18, 2024 02:54
@geoffviola geoffviola self-requested a review December 18, 2024 15:16
Copy link
Contributor

@geoffviola geoffviola left a comment

Choose a reason for hiding this comment

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

Implementation, tests, and docs look good. Just left some minor comments.

}

TEST(MakeCommon, PreservesCategory) {
constexpr auto feeters = make_common(feet, meters);
Copy link
Contributor

Choose a reason for hiding this comment

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

feeters

😆

@hoffbrinkle hoffbrinkle self-requested a review December 18, 2024 16:08
chiphogg and others added 3 commits December 18, 2024 11:40
Co-authored-by: Michael Hordijk <hoffbrinkle@gmail.com>
@chiphogg chiphogg merged commit 3074486 into main Dec 18, 2024
14 checks passed
@chiphogg chiphogg deleted the chiphogg/make-common branch December 18, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: 📝 documentation PR affecting library documentation release notes: ✨ lib (enhancement) PR enhancing the library code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants