Skip to content

Conversation

@ednolan
Copy link
Member

@ednolan ednolan commented Jan 4, 2026

No description provided.

@ednolan ednolan requested a review from tzlaine as a code owner January 4, 2026 20:34
@ednolan ednolan force-pushed the enolan_exemplarify2 branch from ac3f091 to 7bedbab Compare January 4, 2026 20:37
@coveralls
Copy link

coveralls commented Jan 4, 2026

Coverage Status

coverage: 100.0%. remained the same
when pulling e63d5d2 on enolan_exemplarify2
into e5d92f5 on main.

@ednolan ednolan force-pushed the enolan_exemplarify2 branch 2 times, most recently from 756dedc to 0f8f41d Compare January 4, 2026 23:24
@ednolan ednolan force-pushed the enolan_exemplarify2 branch from 0f8f41d to f5507d4 Compare January 4, 2026 23:38

<details>
<summary> Use beman.transform_view directly from C++ </summary>

Copy link
Member

Choose a reason for hiding this comment

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

The statement below is valid -- we really need to separate the readme for building versus 'using in your project'.

Copy link
Member

Choose a reason for hiding this comment

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

Let me clarify what I'm talking about:

This library is header only.  If you want to use `beman.transform_view` from your
project, you can include `beman/transform_view/*.hpp` files from your C++ source
files

For me this is a valid way to use the library -- especially if you're using a non-cmake build system. It's a very common way to use header only libraries because it's easy and portable. We shouldn't discourage it just because some of our core members don't want to consume libraries this way.

The more general issue is that the examplar 'readme outline' is a mess. In my view all this 'building for developmentsort of text shouldn't be mixed in with the 'how do I just use this in my project: aka conan, vcpkg, etc. I think @ednolan has mentioned that thebuilding for development` should probably be a linked .md file so that the primary markdown can be radically simplified and less overwhelming.

So we don't need to fix it here now -- but it's bugging me :)

Copy link
Collaborator

@tzlaine tzlaine left a comment

Choose a reason for hiding this comment

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

This all seems fine to me. Are Jeff's comments blocking this? I'm unclear if I should merge it as-is.

Copy link
Member

@JeffGarland JeffGarland left a comment

Choose a reason for hiding this comment

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

I merged the one thing that really needed changing -- the unconditional requirement of ctest even when not building tests.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@JeffGarland JeffGarland merged commit fae6e91 into main Jan 25, 2026
50 checks passed
@ednolan ednolan deleted the enolan_exemplarify2 branch January 25, 2026 21:09
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.

5 participants