Skip to content

Conversation

@ckormanyos
Copy link
Contributor

Checklist

  • [x ] I have read the CONTRIBUTING guidelines
  • [x ] The coding style is consistent with the rest of the library
  • [x ] My branch's history is clean and coherent. This could be done through
    at least one of the following practices:
    • Rebasing my branch off of the branch being merged to
    • Squashing commits to create a more cohesive history
  • [x ] If relevant, I have included unit-tests (for new code/bugfixes)

Description

GitHub Issues

@ckormanyos ckormanyos requested a review from bitwizeshift as a code owner May 6, 2025 17:43
@ckormanyos ckormanyos changed the title Fix recursive cend() via known line from above Fixes #26 recursive cend() with line from above May 6, 2025
@ckormanyos
Copy link
Contributor Author

Hi @bitwizeshift I've PR'ed this simple fix of the recursive call expressed in #26.

I have not looked at your CI, but some of those runners will (if they are the default on GHA) be long deprecated.

Cc: @arjunae

Copy link
Owner

@bitwizeshift bitwizeshift left a comment

Choose a reason for hiding this comment

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

Hi @bitwizeshift I've PR'ed this simple fix of the recursive call expressed in #26.

I have not looked at your CI, but some of those runners will (if they are the default on GHA) be long deprecated.

Cc: @arjunae

Thanks for the contribution!

Yep, these are very deprecated -- but I'm not entirely surprised. I always meant to convert these to docker containers, just never got around to it.

@bitwizeshift bitwizeshift merged commit 169d7fa into bitwizeshift:master May 6, 2025
3 of 54 checks passed
@bitwizeshift
Copy link
Owner

Bypassed checks because of failing runners. Thanks again!

@ckormanyos
Copy link
Contributor Author

but I'm not entirely surprised. I always meant to convert these to docker containers, just never got around to it.

Indeed, yours is an interesting case. At first, I thought (naively) hey, update to latest whatever.

But that would not be right. The whole point of the Backport is to NOT use stuff like ubuntu-latest or windows-latest or macos-latest. This is because you are specifically targeting old compilers.

At Boost where I work also, we have a bunch of containers that have stuff back to the era of GCC-5. But I'm not really responsible of the containers in the projects where I work.

@ckormanyos
Copy link
Contributor Author

Thanks @bitwizeshift and @arjunae

@ckormanyos ckormanyos deleted the recursive_cend branch May 6, 2025 17:53
@bitwizeshift
Copy link
Owner

@ckormanyos

But that would not be right. The whole point of the Backport is to NOT use stuff like ubuntu-latest or windows-latest or macos-latest. This is because you are specifically targeting old compilers.

Yeah, that's exactly the problem.

That said: I have a lot more CI experience nowadays; certainly much more than I did when I started this project, so this is actually something that I should try getting to. GitHub supports using docker container images as the base runner image -- so in theory I should be able to use some of gcc and clang containers for that. The only thing I would need to do is install cmake in those containers. Of course, this will only work for Linux. MacOS and Windows I'd suspect shouldn't matter as much.

I just haven't done C++ in ages, so this project (and my other C++ projects) have fallen on the back-burner. I'll try to get around to this this weekend.

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.

2 participants