-
Notifications
You must be signed in to change notification settings - Fork 82
refactor(cmake): Rename CMake dependency variables to align with the latest dependency directory layout. #1353
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
Conversation
WalkthroughRenames CMake variables from CLP_CORE_DEPS_DIR to CLP_DEPS_CPP_DIR and updates related include and conditional references in core build scripts and the FindLibLZMA module. Adjusts PKG_CONFIG_PATH construction accordingly. No changes to exported/public entities or external behaviour. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other all lgtm. The only question is: shouldn't CLP_CPP_DEPS_DIR
make more sense? I understand CLP_DEPS_CORE_DIR
would match the hierarchical structure of the actual directory, but it reads a bit weird though.
I think I want to match how it's done in taskfiles, e.g. Line 10 in 62bea59
Before your PR I always wanted to change the original In short, want to align CMake variable naming with Task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. For the PR title, how about:
refactor(cmake): Rename CMake dependency variables to align with the latest dependency directory layout.
Description
Following:
deps:core
anddeps:spider-dep
write into different top-level CMake configuration files (fixes #1330). #1331This PR renames the remaining CMake variables that referenced the old deps directory layout so they accurately reflect the new
build/deps/cpp/cmake-settings/all-core.cmake
structure.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Refactor
Chores