Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Remove vcpkg requirement and use conda for range-v3 #1835

Merged
merged 4 commits into from
Oct 18, 2023

Conversation

horizon-blue
Copy link
Contributor

@horizon-blue horizon-blue commented Oct 17, 2023

Motivation

Now that range-v3 is available in conda-forge (conda-forge/staged-recipes#22443), we can simplify the installation and get rid of the need to install vcpkg just to get the range-v3 dependency.

Changes proposed

  • Remove vcpkg installation guide from README and CI
  • Install range-v3 from conda-forge together when we install Boost and Eigen

Test Plan

I have tested and verify that the changes run on my MacOS machine (with Python 3.10). If the CI also pass then we should be good.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • The title of my pull request is a short description of the requested changes.

Now that range-v3 is available in conda-forge, we could simplify the installation and get rid of the need to install vcpkg
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 17, 2023
Copy link
Contributor

@ndmlny-qs ndmlny-qs left a comment

Choose a reason for hiding this comment

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

super! thanks for the fix

@ndmlny-qs
Copy link
Contributor

@horizon-blue if there are no other items you want to add to the PR, then let me know if you can do a squash/merge. If not, I think I might be able to fix this for future PRs.

@horizon-blue horizon-blue merged commit 5c1da0a into main Oct 18, 2023
9 checks passed
@horizon-blue horizon-blue deleted the simplify-installation branch October 18, 2023 16:25
@horizon-blue
Copy link
Contributor Author

Looks like I am able to squash and merge just fine. Thanks for the review, @ndmlny-qs!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants