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

Migrate CMake deps to FetchContent and more std:: usage #84

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ilelann
Copy link
Contributor

@ilelann ilelann commented Jul 10, 2021

getting rid of setup_dep CMake helper

also contains some s/std/boost/ to remove few boost submodules
std::thread
std::tuple
std::chrono
std::bind (but boost::bind is still required indirectly)

at last, some minor fixes to compile without warning

getting rid of setup_dep CMake helper

also contains some s/std/boost/ to remove few boost submodules
  std::thread
  std::tuple
  std::chrono
  std::bind (but boost::bind is still required indirectly)

at last, some minor fixes to compile without warning
@@ -17,11 +17,7 @@
#include <unordered_map>
#include <vector>

#ifdef ADOBE_BUILT_WITH_CMAKE
#include <double-conversion/double-conversion.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure how non-CMake build setups will react to that :)

Copy link
Contributor

@camio camio left a comment

Choose a reason for hiding this comment

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

Could you please break this PR into multiple PRs? As it stands it is too big to reasonably review. For example, the migration of CMake deps to FetchContent is stand alone. As is each of the migrations to std from boost.

build_asl/

# CLion
cmake-build-*/
Copy link
Contributor

Choose a reason for hiding this comment

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

IDE and editor-specific files should be added to global .gitignore files instead of project ones. See this stack overflow article.

@ilelann
Copy link
Contributor Author

ilelann commented Aug 24, 2022

Could you please break this PR into multiple PRs? As it stands it is too big to reasonably review. For example, the migration of CMake deps to FetchContent is stand alone. As is each of the migrations to std from boost.

Agreed. I will split the PR.

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