-
Notifications
You must be signed in to change notification settings - Fork 72
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
fix(core): Add missing ../
to fix relative header file includes.
#627
Conversation
WalkthroughThe pull request introduces significant modifications across several files, primarily focusing on enhancing command-line argument parsing and improving error handling. Key changes include restructuring include paths for header files, refining the parsing logic in Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (4)
🧰 Additional context used📓 Path-based instructions (4)components/core/src/clp/clp/decompression.cpp (1)Pattern components/core/src/clp_s/CommandLineArguments.cpp (1)Pattern components/core/src/clp/clp/utils.cpp (1)Pattern components/core/src/clp/clo/CommandLineArguments.cpp (1)Pattern 🔇 Additional comments (4)components/core/src/clp/clo/CommandLineArguments.cpp (1)
The include path has been updated to components/core/src/clp_s/CommandLineArguments.cpp (1)
The inclusion of components/core/src/clp/clp/utils.cpp (1)
The include paths for components/core/src/clp/clp/decompression.cpp (1)
The include path has been changed to Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
For PR title, how about:
fix(core): Add missing `../` to fix relative header file includes.
Also, please remember to add a period at the end of the PR title :D
../
to fix relative header file includes.
…-scope#627) Co-authored-by: Bingran Hu <[email protected]>
Description
In the old structure of
clp-core
, individual modules likeclp-s
,glt
andreducer
are all under the same path as the rest of theclp-core
source files, which is<project>/components/core/src
.After encapsulating the rest of the files into a single
clp
module, there are now four modules under<project>/components/core/src
:clp
,clp-s
,glt
andreducer
.If files inside the
clp
module need to refer to files in the other three modules, their#include
paths will need to move up an extra level.There are also other scenarios where the header files need updates, and they are all included in this PR.
Discovery
As I was working on porting utility libraries like
string_utils
andregex_utils
to a new repo, I found out that removing these folders created compilation errors. Upon further investigation, these libraries' CMake files contain the following line:target_include_directories(<utils_library_name> PUBLIC ../)
where
../
is the path<project_root>/components/core/src/clp
Without this line,
string_utils
andregex_utils
cannot be found. But with this line, files (that are not meant to be libraries) under<project_root>/components/core/src/clp
can be included without specifying the correct relative path.After removing these library folders and fixing all
string_utils
andregex_utils
include paths, the remaining compilation errors would all be due to the incorrect relative paths. This PR fixes all these incorrect includes.Fixing these include paths won't affect anything for now, but it will simply PRs down the road when we are ready to move
string_utils
andregex_utils
out ofclp-core
.Validation performed
All compilations succeed after performing the operations described above, and unit tests have passed as usual.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores