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

Drop range-v3 and replace it with std::ranges #1611

Open
christianparpart opened this issue Oct 1, 2024 · 8 comments
Open

Drop range-v3 and replace it with std::ranges #1611

christianparpart opened this issue Oct 1, 2024 · 8 comments
Labels
code health Code quality related topics.

Comments

@christianparpart
Copy link
Member

We want to reduce external dependencies and embrace standards.

C++20 comes with its own <ranges> library, so there is no need to depend on range-v3 library anymore.

Most places should be trivial to adapt. If any first-time contributor comes across this ticket, don't hesitate to talk to us on Discord, so we can help you getting on board and guide you through.

@christianparpart christianparpart added help wanted Extra attention is needed good first issue Good for newcomers code health Code quality related topics. labels Oct 1, 2024
@DIlkhush00
Copy link

Hi @christianparpart , I would love to tackle this issue. Could you please provide more details on it?

@christianparpart
Copy link
Member Author

christianparpart commented Oct 1, 2024

Hi @christianparpart , I would love to tackle this issue. Could you please provide more details on it?

Oh this is great to hear. I think the following is the action list that needed to be done:

  1. get rid of the range-v3 dependency
  2. replace ranges namespace with std::ranges

Getting rid of the dependency

If you do a git grep range-v3 on the command line, you should get the list of files that need to be touched:

  • /scripts/install-deps.sh
  • /vcpkg.json
  • /cmake/ContourThirdParties.cmake
  • various CMakeLists.txt files
  • /src/contour/ContourApp.cpp (mentions range-v3 as project we use)

All of those files should simply get the occurrence removed. (In case of problems, just ask us)

Replacing the actual code calls

This should(tm) be as simple as replacing all ::ranges:: with std::ranges::. But of course, there might be some corner cases that are not that trivial.

Keep in mind, that Standard C++20 also provides a shortcut for std::ranges::views to std::views, that might make it easier to look at and use/type.

p.s.: If you create a PR early, we can also give early feedback. You don't have to wait until it is fully working, in case it might take you longer. Keep in mind, you can also join us on Discord for (most of the time) realtime chatting, in case it might help.

Good luck!
And many thanks.

@EKS2003
Copy link

EKS2003 commented Oct 1, 2024

Hey! Has this been fixed? I would like to give it a try.

@christianparpart
Copy link
Member Author

Hey @EKS2003. It's a pleasure to get such a positive feedback from all of you, guys. However, this ticket has already been claimed. I can recommend you #1613, even though, this is a really trivial one (I'm sorry), it yet has to be done.

Apart from that, i sadly do not know how much you want to dive in, but we could surely find more, when you join us on Discord, so we can figure a good and easy ticket out for you.

Many thanks from all of you.

@DIlkhush00
Copy link

Hi @christianparpart , something urgent has come up, and I won't be able to work on this issue at the moment. It's great that there's already someone claiming it. You can go ahead and assign it to @EKS2003

@christianparpart
Copy link
Member Author

@DIlkhush00 I'm sorry to hear that. Many thanks for informing us :)

@Yaraslaut
Copy link
Member

Hi @EKS2003, is there any updates?

@Yaraslaut Yaraslaut removed help wanted Extra attention is needed good first issue Good for newcomers labels Oct 17, 2024
@Yaraslaut
Copy link
Member

Some of things that we use is from c++23 standard, for example
https://en.cppreference.com/w/cpp/ranges/to
https://en.cppreference.com/w/cpp/ranges/join_with_view
https://en.cppreference.com/w/cpp/ranges/enumerate_view

I suppose it is better to wait until we switch to c++23 and make this transition, at the moment we compile on mac with clang-15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Code quality related topics.
Projects
None yet
Development

No branches or pull requests

4 participants