feat(tdl): Add Spider TDL component; Add boilerplate for TDL parser's AST node. - #175
Merged
Merged
Conversation
Contributor
WalkthroughA new library target, Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase as test-parser-ast.cpp
participant Node as spider::tdl::parser::ast::Node
participant ErrorHandling as ystdlib::error_handling
TestCase->>Node: Create ErrorCode with PlaceholderError
TestCase->>ErrorHandling: Wrap ErrorCode in Result<void>
TestCase->>ErrorHandling: Query Result<void> for error state
ErrorHandling-->>TestCase: Returns has_error() == true
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/spider/CMakeLists.txt(1 hunks)src/spider/tdl/parser/ast/Node.cpp(1 hunks)src/spider/tdl/parser/ast/Node.hpp(1 hunks)tests/CMakeLists.txt(2 hunks)tests/tdl/test-parser-ast.cpp(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: davidlion
PR: y-scope/spider#100
File: src/spider/worker/worker.cpp:205-230
Timestamp: 2025-04-09T17:15:24.552Z
Learning: Documentation should be added to new functions in the spider codebase, as already discussed with the user.
Learnt from: sitaowang1998
PR: y-scope/spider#172
File: build-tasks.yaml:21-45
Timestamp: 2025-08-03T01:49:27.764Z
Learning: In the spider project, external tools like `uv`, Python, and Task are documented as requirements in README.md and are expected to be pre-installed by users rather than bootstrapped by the build system.
Learnt from: sitaowang1998
PR: y-scope/spider#168
File: lint-tasks.yaml:62-65
Timestamp: 2025-07-17T19:44:06.132Z
Learning: ANTLR-generated C++ code in the G_SRC_DSL_DIR (src/stdl) should not be included in linting tasks because it's auto-generated code that doesn't follow manual coding standards.
📚 Learning: antlr-generated c++ code in the g_src_dsl_dir (src/stdl) should not be included in linting tasks bec...
Learnt from: sitaowang1998
PR: y-scope/spider#168
File: lint-tasks.yaml:62-65
Timestamp: 2025-07-17T19:44:06.132Z
Learning: ANTLR-generated C++ code in the G_SRC_DSL_DIR (src/stdl) should not be included in linting tasks because it's auto-generated code that doesn't follow manual coding standards.
Applied to files:
tests/CMakeLists.txtsrc/spider/CMakeLists.txt
📚 Learning: in the spider project, internal dependencies between packages (like spider-client depending on spide...
Learnt from: sitaowang1998
PR: y-scope/spider#172
File: python/spider/client/pyproject.toml:6-7
Timestamp: 2025-08-03T01:52:26.847Z
Learning: In the spider project, internal dependencies between packages (like spider-client depending on spider-core) are managed without version pinning since they are developed together in the same repository and versions are kept synchronized.
Applied to files:
tests/CMakeLists.txtsrc/spider/CMakeLists.txt
📚 Learning: documentation should be added to new functions in the spider codebase, as already discussed with the...
Learnt from: davidlion
PR: y-scope/spider#100
File: src/spider/worker/worker.cpp:205-230
Timestamp: 2025-04-09T17:15:24.552Z
Learning: Documentation should be added to new functions in the spider codebase, as already discussed with the user.
Applied to files:
src/spider/CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: non-storage-unit-tests (ubuntu-22.04)
- GitHub Check: non-storage-unit-tests (ubuntu-24.04)
- GitHub Check: lint
🔇 Additional comments (4)
tests/CMakeLists.txt (1)
5-5: LGTM! Clean integration of TDL test component.The addition of the new test source file and library dependency follows the established patterns in the CMake configuration and properly integrates the TDL component into the test build system.
Also applies to: 43-43
tests/tdl/test-parser-ast.cpp (1)
1-19: LGTM! Well-structured unit test for AST node error handling.The test properly validates the integration between the Node class and the error handling system. The comprehensive NOLINT directives and clean test structure follow established patterns in the codebase.
src/spider/tdl/parser/ast/Node.hpp (1)
9-22: LGTM! Appropriate foundational AST node structure.The Node class provides a solid foundation for the TDL parser AST with proper error handling integration. The current minimal implementation is appropriate for boilerplate/foundational code as mentioned in the PR objectives.
src/spider/CMakeLists.txt (1)
198-217: LGTM! Well-structured CMake configuration for the new TDL library.The spider_tdl library definition follows established patterns in the project, with proper separation of sources and headers, appropriate PUBLIC linkage for ystdlib::error_handling, and consistent alias target creation.
sitaowang1998
approved these changes
Aug 5, 2025
This was referenced Aug 7, 2025
This was referenced Aug 20, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR serves as a boilerplate for TDL (task definition language) as a new component for Spider, with a simple declaration for its parser AST node.
This component is presented as a new target in the cmake,
spider_tdl. We introduce a simple unit test case to ensure this component can be successfully linked by the unit test executable.Checklist
breaking change.
Validation performed
Summary by CodeRabbit