Skip to content

ci: Parallelize clang-tidy - #94

Closed
sitaowang1998 wants to merge 10 commits into
y-scope:mainfrom
sitaowang1998:clang-tidy
Closed

ci: Parallelize clang-tidy#94
sitaowang1998 wants to merge 10 commits into
y-scope:mainfrom
sitaowang1998:clang-tidy

Conversation

@sitaowang1998

@sitaowang1998 sitaowang1998 commented Apr 4, 2025

Copy link
Copy Markdown
Collaborator

Description

Currently a run of clang-tidy on all files take at least 70 minutes in GH workflow. This pr updates yscope-dev-utils to d22183f and incorporates its parallelized clang-tidy tasks in GH workflow.

This pr also includes the changes in source files according to the latest clang-format rules.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • GitHub workflow pass
  • clang-format and clang-tidy checks pass in dev container

Summary by CodeRabbit

  • Chores

    • Improved the linting workflow by enabling parallel processing and upgrading tool dependencies for better performance.
  • Refactor

    • Streamlined and consolidated configuration tasks in automated workflows to enhance system efficiency.
  • Style

    • Applied consistent formatting improvements across the codebase for improved readability and maintainability.

These internal enhancements aim to support a more robust development process while ensuring our system continues to run smoothly.

@sitaowang1998
sitaowang1998 requested a review from a team as a code owner April 4, 2025 02:39
@coderabbitai

coderabbitai Bot commented Apr 4, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request updates various linting and build configuration files while applying widespread cosmetic changes to C++ source and header files. The linting workflow now uses a command that leverages all available CPU cores, and dependency versions have been updated. Additionally, multiple task definitions and paths in YAML configuration files have been reorganized. In the C++ code, the modifications consist chiefly of formatting adjustments such as the removal of extraneous blank lines and reformatting of function signatures to improve readability, without altering the underlying functionality.

Changes

File(s) Change Summary
.github/workflows/code-linting-checks.yaml
lint-requirements.txt
lint-tasks.yaml
taskfile.yaml
test-tasks.yaml
Updated linting command to use multi-core processing, updated clang-format dependency to version ≥20.1.0, adjusted lint task configurations, and modified paths and variable assignments in task configurations.
src/spider/...
tests/...
Numerous cosmetic and formatting improvements across C++ source and header files including removal of extraneous blank lines, reformatting of function/method signatures (e.g. moving return types, consolidating lines), and overall code style consistency improvements without functional changes.
tools/yscope-dev-utils Subproject commit reference updated to a new commit hash.

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sitaowang1998
sitaowang1998 requested a review from davidlion April 4, 2025 02:41

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
src/spider/io/msgpack_message.cpp (1)

241-248: ⚠️ Potential issue

Potential error code mismatch in asynchronous message reading.

In the receive_message_async function, after reading the message body, the error check currently compares boost::asio::error::eof with body_size_ec rather than with body_ec. This appears to be an oversight; the comparison should be made with body_ec to ensure correct error handling.

Suggested diff:

-        if (boost::asio::error::eof != body_size_ec) {
+        if (boost::asio::error::eof != body_ec) {
🧹 Nitpick comments (2)
src/spider/client/Job.hpp (1)

226-229: Consistent Exception Formatting for Insufficient Output Tasks
The reformatting of the exception thrown for insufficient output tasks now spans multiple lines, which noticeably improves readability and clearly separates the exception construction from the formatting logic. This style is both modern and consistent with similar changes elsewhere in the file.

src/spider/storage/StorageFactory.hpp (1)

18-23: Enhanced Signature Formatting

The reformatting of the method declarations for provide_storage_connection and provide_job_submission_batch significantly improves readability by placing the return types on the same line as the method names. This change is stylistic only and does not affect the functionality.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb7ec79 and 1fe339a.

📒 Files selected for processing (74)
  • .github/workflows/code-linting-checks.yaml (1 hunks)
  • lint-requirements.txt (1 hunks)
  • lint-tasks.yaml (5 hunks)
  • src/spider/client/Data.hpp (0 hunks)
  • src/spider/client/Driver.cpp (0 hunks)
  • src/spider/client/Driver.hpp (2 hunks)
  • src/spider/client/Job.hpp (3 hunks)
  • src/spider/client/TaskContext.cpp (0 hunks)
  • src/spider/client/TaskContext.hpp (2 hunks)
  • src/spider/client/task.hpp (0 hunks)
  • src/spider/client/type_utils.hpp (0 hunks)
  • src/spider/core/Data.hpp (0 hunks)
  • src/spider/core/DataImpl.hpp (0 hunks)
  • src/spider/core/Driver.hpp (0 hunks)
  • src/spider/core/Error.hpp (0 hunks)
  • src/spider/core/JobMetadata.hpp (0 hunks)
  • src/spider/core/Task.hpp (2 hunks)
  • src/spider/core/TaskContextImpl.hpp (1 hunks)
  • src/spider/core/TaskGraph.hpp (4 hunks)
  • src/spider/core/TaskGraphImpl.hpp (3 hunks)
  • src/spider/io/msgpack_message.cpp (2 hunks)
  • src/spider/io/msgpack_message.hpp (1 hunks)
  • src/spider/scheduler/FifoPolicy.cpp (2 hunks)
  • src/spider/scheduler/FifoPolicy.hpp (0 hunks)
  • src/spider/scheduler/SchedulerMessage.hpp (0 hunks)
  • src/spider/scheduler/SchedulerPolicy.hpp (1 hunks)
  • src/spider/scheduler/SchedulerServer.cpp (1 hunks)
  • src/spider/scheduler/SchedulerServer.hpp (0 hunks)
  • src/spider/scheduler/scheduler.cpp (2 hunks)
  • src/spider/storage/DataStorage.hpp (1 hunks)
  • src/spider/storage/MetadataStorage.hpp (1 hunks)
  • src/spider/storage/StorageConnection.hpp (0 hunks)
  • src/spider/storage/StorageFactory.hpp (1 hunks)
  • src/spider/storage/mysql/MySqlConnection.cpp (1 hunks)
  • src/spider/storage/mysql/MySqlConnection.hpp (1 hunks)
  • src/spider/storage/mysql/MySqlJobSubmissionBatch.cpp (0 hunks)
  • src/spider/storage/mysql/MySqlJobSubmissionBatch.hpp (0 hunks)
  • src/spider/storage/mysql/MySqlStorage.cpp (5 hunks)
  • src/spider/storage/mysql/MySqlStorage.hpp (5 hunks)
  • src/spider/storage/mysql/MySqlStorageFactory.cpp (2 hunks)
  • src/spider/storage/mysql/MySqlStorageFactory.hpp (1 hunks)
  • src/spider/utils/LruCache.hpp (0 hunks)
  • src/spider/worker/DllLoader.cpp (0 hunks)
  • src/spider/worker/DllLoader.hpp (0 hunks)
  • src/spider/worker/FunctionManager.cpp (2 hunks)
  • src/spider/worker/FunctionManager.hpp (4 hunks)
  • src/spider/worker/FunctionNameManager.cpp (0 hunks)
  • src/spider/worker/FunctionNameManager.hpp (0 hunks)
  • src/spider/worker/Process.cpp (0 hunks)
  • src/spider/worker/Process.hpp (0 hunks)
  • src/spider/worker/TaskExecutor.cpp (1 hunks)
  • src/spider/worker/TaskExecutor.hpp (0 hunks)
  • src/spider/worker/TaskExecutorMessage.hpp (0 hunks)
  • src/spider/worker/WorkerClient.cpp (1 hunks)
  • src/spider/worker/WorkerClient.hpp (1 hunks)
  • src/spider/worker/message_pipe.cpp (1 hunks)
  • src/spider/worker/message_pipe.hpp (1 hunks)
  • src/spider/worker/task_executor.cpp (0 hunks)
  • src/spider/worker/worker.cpp (7 hunks)
  • taskfile.yaml (1 hunks)
  • test-tasks.yaml (1 hunks)
  • tests/client/client-test.cpp (0 hunks)
  • tests/client/test-Driver.cpp (0 hunks)
  • tests/io/test-MsgpackMessage.cpp (0 hunks)
  • tests/scheduler/test-SchedulerServer.cpp (0 hunks)
  • tests/storage/StorageTestHelper.hpp (0 hunks)
  • tests/storage/test-DataStorage.cpp (0 hunks)
  • tests/storage/test-MetadataStorage.cpp (2 hunks)
  • tests/utils/CoreDataUtils.hpp (0 hunks)
  • tests/utils/CoreTaskUtils.cpp (0 hunks)
  • tests/utils/CoreTaskUtils.hpp (0 hunks)
  • tests/worker/test-Process.cpp (0 hunks)
  • tests/worker/test-TaskExecutor.cpp (1 hunks)
  • tools/yscope-dev-utils (1 hunks)
💤 Files with no reviewable changes (36)
  • src/spider/worker/Process.hpp
  • src/spider/storage/mysql/MySqlJobSubmissionBatch.cpp
  • src/spider/worker/TaskExecutorMessage.hpp
  • src/spider/core/Data.hpp
  • src/spider/worker/DllLoader.hpp
  • src/spider/core/JobMetadata.hpp
  • tests/utils/CoreTaskUtils.cpp
  • src/spider/scheduler/SchedulerServer.hpp
  • tests/client/client-test.cpp
  • src/spider/worker/FunctionNameManager.cpp
  • tests/io/test-MsgpackMessage.cpp
  • src/spider/scheduler/FifoPolicy.hpp
  • src/spider/worker/task_executor.cpp
  • src/spider/client/type_utils.hpp
  • tests/storage/test-DataStorage.cpp
  • src/spider/worker/Process.cpp
  • src/spider/worker/FunctionNameManager.hpp
  • src/spider/client/Data.hpp
  • src/spider/utils/LruCache.hpp
  • tests/client/test-Driver.cpp
  • src/spider/client/Driver.cpp
  • tests/worker/test-Process.cpp
  • src/spider/core/Error.hpp
  • src/spider/worker/DllLoader.cpp
  • tests/storage/StorageTestHelper.hpp
  • tests/utils/CoreDataUtils.hpp
  • src/spider/scheduler/SchedulerMessage.hpp
  • src/spider/storage/mysql/MySqlJobSubmissionBatch.hpp
  • src/spider/core/DataImpl.hpp
  • tests/scheduler/test-SchedulerServer.cpp
  • src/spider/core/Driver.hpp
  • src/spider/client/task.hpp
  • tests/utils/CoreTaskUtils.hpp
  • src/spider/storage/StorageConnection.hpp
  • src/spider/worker/TaskExecutor.hpp
  • src/spider/client/TaskContext.cpp
🧰 Additional context used
🧬 Code Definitions (9)
src/spider/scheduler/scheduler.cpp (1)
src/spider/storage/MetadataStorage.hpp (16)
  • conn (27-27)
  • conn (29-29)
  • conn (30-30)
  • conn (32-33)
  • conn (37-40)
  • conn (42-48)
  • conn (50-51)
  • conn (53-54)
  • conn (56-57)
  • conn (59-63)
  • conn (66-67)
  • conn (69-73)
  • conn (75-75)
  • conn (76-76)
  • conn (77-78)
  • conn (80-80)
src/spider/scheduler/FifoPolicy.cpp (1)
src/spider/scheduler/FifoPolicy.hpp (1)
  • worker_addr (32-32)
src/spider/storage/mysql/MySqlConnection.cpp (1)
src/spider/storage/mysql/MySqlConnection.hpp (1)
  • url (34-35)
src/spider/client/TaskContext.hpp (2)
src/spider/core/TaskGraph.hpp (4)
  • task (22-38)
  • task (22-22)
  • task (41-48)
  • task (41-41)
src/spider/core/TaskGraphImpl.hpp (2)
  • task (322-344)
  • task (322-322)
src/spider/worker/WorkerClient.cpp (1)
src/spider/worker/WorkerClient.hpp (1)
  • fail_task_id (34-35)
src/spider/client/Driver.hpp (2)
src/spider/core/TaskGraphImpl.hpp (2)
  • task (322-344)
  • task (322-322)
src/spider/core/TaskGraph.hpp (4)
  • task (22-38)
  • task (22-22)
  • task (41-48)
  • task (41-41)
src/spider/storage/mysql/MySqlStorage.hpp (1)
src/spider/storage/MetadataStorage.hpp (16)
  • conn (27-27)
  • conn (29-29)
  • conn (30-30)
  • conn (32-33)
  • conn (37-40)
  • conn (42-48)
  • conn (50-51)
  • conn (53-54)
  • conn (56-57)
  • conn (59-63)
  • conn (66-67)
  • conn (69-73)
  • conn (75-75)
  • conn (76-76)
  • conn (77-78)
  • conn (80-80)
src/spider/core/Task.hpp (2)
src/spider/core/Data.hpp (8)
  • Data (14-14)
  • Data (16-16)
  • Data (16-16)
  • Data (18-18)
  • nodiscard (20-20)
  • nodiscard (22-22)
  • nodiscard (24-26)
  • nodiscard (28-28)
src/spider/core/TaskGraph.hpp (8)
  • nodiscard (54-59)
  • nodiscard (61-66)
  • nodiscard (68-77)
  • nodiscard (79-88)
  • nodiscard (91-94)
  • nodiscard (98-100)
  • nodiscard (102-104)
  • nodiscard (110-113)
src/spider/storage/MetadataStorage.hpp (6)
src/spider/client/Job.hpp (2)
  • conn (180-198)
  • status (91-123)
src/spider/core/Task.hpp (4)
  • client_id (148-148)
  • client_id (148-148)
  • id (186-186)
  • id (186-186)
src/spider/storage/mysql/MySqlStorage.hpp (1)
  • batch (127-132)
src/spider/core/TaskGraph.hpp (12)
  • id (54-54)
  • id (61-61)
  • id (68-69)
  • id (79-80)
  • id (106-106)
  • id (106-106)
  • id (108-108)
  • id (108-108)
  • task (22-38)
  • task (22-22)
  • task (41-48)
  • task (41-41)
src/spider/client/Driver.hpp (1)
  • job_ids (309-317)
src/spider/core/TaskGraphImpl.hpp (2)
  • task (322-344)
  • task (322-322)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: non-storage-unit-tests (ubuntu-22.04)
  • GitHub Check: non-storage-unit-tests (ubuntu-24.04)
  • GitHub Check: lint
🔇 Additional comments (84)
src/spider/client/Job.hpp (2)

270-273: Enhanced Error Reporting in Msgpack Unpack Catch Block
The multi-line formatting of the thrown exception in the catch block for msgpack::type_error significantly improves clarity. The separation of the exception keyword and the formatted message helps ensure that the error details are immediately apparent upon inspection.


307-310: Uniform Exception Handling for Data Retrieval Failures
The updated formatting when throwing a ConnectionException after a failed data retrieval is consistent with other exception cases in this file. This uniformity aids maintainability and enhances the overall readability of error handling throughout the function.

tests/worker/test-TaskExecutor.cpp (1)

1-250: Overall, the test suite is well-structured and continues to validate TaskExecutor functionality correctly.

The formatting and style improvements enhance readability without changing test logic.

🧰 Tools
🪛 Cppcheck (2.10-2)

[error] 63-63: syntax error

(syntaxError)

src/spider/worker/TaskExecutor.cpp (1)

129-137: Refactored get_error method formatting for clarity.

The return statement for get_error has been reformatted into a more compact, single-line style (spread over multiple indented lines) without altering its functionality. This change improves readability and aligns with the modern C++ style adopted elsewhere in the file.

src/spider/worker/WorkerClient.hpp (1)

34-36: Updated get_next_task method signature formatting.

The declaration of get_next_task has been adjusted so that the return type is on the same line as the parameter list. This enhances consistency and readability across the codebase.

src/spider/io/msgpack_message.cpp (1)

140-142: Improved formatting for reading body size.

The multi-line call to read_ext_body_size now clearly shows the conversion of the body size vector to a std::u8string_view, thereby improving the readability of the code.

src/spider/io/msgpack_message.hpp (1)

20-22: Refactored receive_message_async declaration for consistent formatting.

The updated declaration now places the return type on the same line as the function signature, aligning it with the implementation and making the code easier to read.

src/spider/worker/message_pipe.cpp (1)

87-88: Formatting Improvement for receive_message_async:
The updated function signature now places the return type immediately after the parameter list, which improves readability. Please ensure that the corresponding declaration in the header file stays consistent with this change.

src/spider/worker/WorkerClient.cpp (1)

44-45: Signature Formatting Update for get_next_task:
The revised signature for get_next_task now neatly aligns the closing parenthesis with the return type. This change enhances code readability without affecting functionality.

src/spider/worker/message_pipe.hpp (1)

16-17: Improved Declaration Readability for receive_message_async:
The function declaration now uses a single-line format for the return type, which increases clarity. The change is purely stylistic and aligns nicely with the implementation.

src/spider/worker/FunctionManager.cpp (2)

17-18: Signature Refinement for response_get_error:
The updated formatting moves the return type to the same logical block as the parameter list. This consistent style improves readability and maintains clarity in the code.


66-67: Consistent Formatting for response_get_result_buffers:
The revised signature now places the return type directly after the parameter list, aligning with the project’s visual style guidelines. There are no functional changes, and the update aids in maintaining a clean codebase.

src/spider/worker/FunctionManager.hpp (3)

90-91: Declaration Update for response_get_error:
The reformatting of the function signature into a more compact layout enhances readability. It is important that both the declaration and the definition remain in sync.


164-165: Formatting Enhancement for response_get_result_buffers:
This updated declaration improves the clarity and consistency of the code style. No functionality has been altered.


220-222: Inline Function Signature Refinement for create_args_request:
The revised inline declaration now clearly presents the return type on the same line, thereby streamlining the visual structure. This change is purely cosmetic and supports overall maintainability.

src/spider/worker/worker.cpp (6)

83-84: Reformatting of the get_environment_variable signature.
The splitting of the return type across separate lines improves clarity and consistency. The changes are purely cosmetic and maintain the same functionality.


144-146: Enhanced readability in the fetch_task function declaration.
Breaking the function declaration into multiple lines makes the signature easier to read and maintain. No functional changes are introduced here.


160-161: Improved clarity in the get_args_buffers function signature.
The reformatting clearly separates the function name and its return type, enhancing legibility without changing any behaviour.


191-193: Readability boost for the parse_outputs function signature.
The adjustment to the function’s declaration by placing the return type on a new line substantially improves formatting, while leaving functionality intact.


304-306: Clear structuring of the storage connection move operation.
The revised formatting for the std::move expression enhances visual clarity and makes it easier to track the extraction of the storage connection.


437-439: Consistent formatting applied in the main function for connection extraction.
The similar reformatting here mirrors the improvements seen in other parts of the file; it increases readability and supports easier future maintenance.

tools/yscope-dev-utils (1)

1-1: Commit Hash Update Verification

The subproject commit has been updated to d22183ffa3fb95745b012eb38a6e13b7e5f06fca, which aligns with the intended improvements for parallelising clang-tidy. Please ensure that any dependent configurations in files such as lint-tasks.yaml and version constraints in lint-requirements.txt have been verified to work correctly with this update.

tests/storage/test-MetadataStorage.cpp (2)

437-438: Formatting Improvement in Task Finish Call

The adjusted formatting of the closing parenthesis for the task_finish call (lines 437–438) improves readability by clearly grouping the call’s parameters. No functionality changes have been introduced.


509-514: Consistent Formatting in Job Reset’s Task Finish Call

The refactoring of the task_finish call in the job reset test (lines 509–514) aligns parameters and the closing parenthesis more clearly, which enhances overall clarity. This change is purely cosmetic and retains the original behaviour.

taskfile.yaml (1)

9-9: Updated Path for Utils in Taskfile

The utils entry in the includes section now points to
tools/yscope-dev-utils/exports/taskfiles/utils/utils.yaml
which reflects the new directory structure for utility files. This update improves consistency across the project.

.github/workflows/code-linting-checks.yaml (1)

52-52: Parallelised Linting Command

The lint job command now includes the -C $(nproc) option, enabling concurrent execution based on available CPU cores. This change should significantly reduce the runtime of the linting process.

lint-requirements.txt (1)

1-7: Updated Linting Tool Version Constraints

The version constraint for clang-format has been updated to require version 20.1.0 or higher, and the dependency for yamllint has been refined. These modifications ensure compatibility with the updated lint tasks and take advantage of improvements in the newer tool versions.

src/spider/scheduler/scheduler.cpp (2)

93-95: Improved Formatting of Storage Connection Retrieval in Heartbeat Loop

The extraction of the storage connection from the variant in the heartbeat loop has been split over multiple lines for enhanced readability. This reformatting does not affect functionality but makes the code easier to follow.


131-133: Enhanced Clarity in Storage Connection Extraction in Cleanup Loop

Similarly, the multi‐line extraction of the storage connection in the cleanup loop (lines 131–133) improves clarity and consistency. The underlying logic remains unaffected.

src/spider/scheduler/SchedulerPolicy.hpp (1)

19-21: Proper reformatting of the schedule_next signature.
The breaking of the return type onto a separate line improves readability while preserving the intended interface.

src/spider/storage/mysql/MySqlStorageFactory.hpp (1)

22-23: Clean signature update for provide_storage_connection.
The changes to the method signature—placing the return type on the same line—enhance clarity and consistency with our style guidelines.

src/spider/scheduler/SchedulerServer.cpp (1)

117-118: Return type reformatting for process_message is correct.
Moving the return type to its own line makes the method signature more legible without affecting the overall functionality.

src/spider/scheduler/FifoPolicy.cpp (2)

28-30: Reformatting of schedule_next signature is appropriate.
Splitting the signature across multiple lines improves readability and aligns with the consistent style applied across the codebase.


43-44: Consistent update for pop_next_task signature.
The adjustment to the return type placement is a clean cosmetic change that enhances the overall clarity of the function declaration.

src/spider/storage/mysql/MySqlConnection.cpp (1)

19-20: Updated function signature for MySqlConnection::create is well structured.
The new formatting—placing the return type on a new line—improves readability while retaining the complete functionality and error handling of the method.

src/spider/client/TaskContext.hpp (2)

132-133: Consistent Formatting for Task Start Method

The updated formatting for the start method (for tasks) now places the return type on the same line as the function declaration, which enhances clarity and consistency with other function definitions.


190-191: Consistent Formatting for Task Graph Start Method

Similarly, the start method for task graphs has been refactored to consolidate its return type with the declaration, ensuring consistency and better readability throughout the code.

test-tasks.yaml (3)

64-67: Updated Task Dependency and Variable Naming

The dependency task has been updated to ":utils:checksum:validate", and the introduction of the INCLUDE_PATTERNS variable (using the output directory) aligns with the overall reorganisation. These changes contribute to a more consistent naming convention across tasks.


69-73: Refined Virtual Environment Creation Task

The change of the task name to ":utils:misc:create-venv" alongside the update of the LABEL variable to "lint" and the switch of REQUIREMENTS_FILE to the lint-specific version demonstrate a clear intent to support linting tasks. This improves clarity and aligns with updated project conventions.


75-78: Consistent Checksum Task Update

The update to ":utils:checksum:compute" and the corresponding variable changes are consistent with earlier updates and reflect the improved task structure.

src/spider/storage/mysql/MySqlConnection.hpp (2)

34-35: Improved Method Declaration Clarity

The reformatting of the static create method signature simplifies the declaration by placing the return type on the same line as the function name. This change promotes consistency and enhances readability without altering functionality.


37-38: Corrected Constructor Definition Syntax

Changing the constructor’s definition from ending with a semicolon to a brace ensures proper C++ syntax and aligns with standard constructor definitions. This correction improves clarity and adheres to coding standards.

src/spider/storage/mysql/MySqlStorage.cpp (4)

229-231: Improved Prepared Statement Formatting in add_task
The reformatting of the prepared statement instantiation (using multi-line breaks for clarity) in the add_task function enhances code readability. The change maintains functional correctness and aligns well with our code style conventions.


740-743: Enhanced SQL Query Formatting for Output Statement
Splitting the SQL query across multiple lines in the output statement of fetch_full_task greatly improves readability without altering the functionality. This formatting change makes it easier to inspect and maintain complex SQL queries.


872-882: Consistent and Clear Implementation of parse_timestamp
The updated formatting for the parse_timestamp function is clear and concise. Using std::get_time with proper error checking provides a robust conversion from a timestamp string to a system clock time point. There are no functional issues; the improvements are purely cosmetic and help maintain consistency.


2248-2252: Readable Multi-line SQL in remove_dangling_data
The multi-line formatting of the SQL statement in remove_dangling_data improves clarity and maintainability. It makes the subqueries more accessible at a glance. It would be worthwhile to monitor the performance of such subqueries, although the current structure is acceptable.

src/spider/client/Driver.hpp (2)

185-190: Improved Declaration Formatting for start(TaskFunction…)
The revised method signature for the start function that accepts a TaskFunction now places the return type on the same line (using the trailing return type syntax). This new formatting increases the readability and consistency of method declarations without impacting functionality.


249-252: Consistent Formatting for start(TaskGraph…)
Similarly, the start method for TaskGraph now has its return type specified immediately after the parameter list. This change standardises the interface and makes the code easier to read and maintain.

src/spider/core/TaskGraphImpl.hpp (3)

31-32: LGTM: Improved method signature formatting

The reformatted method signature with the arrow (->) on a separate line improves readability, especially for complex return types.


102-106: LGTM: Enhanced readability with multi-line formatting

Breaking the set_data_id call into multiple lines improves code readability for this nested expression chain.


136-137: LGTM: Consistent method signature formatting

This change follows the same formatting pattern as other methods, improving consistency throughout the codebase.

src/spider/core/TaskContextImpl.hpp (2)

29-30: LGTM: Improved method signature formatting

The reformatted method signature enhances readability by placing the return type on a separate line.


34-35: LGTM: Consistent method signature formatting

This formatting change maintains consistency with other method signatures in the codebase.

src/spider/storage/mysql/MySqlStorageFactory.cpp (2)

28-29: LGTM: Enhanced method signature readability

The reformatted method signature improves readability for this complex return type.


38-39: LGTM: Consistent method signature formatting

This change follows the same formatting pattern applied to other methods throughout the codebase.

src/spider/core/TaskGraph.hpp (4)

68-69: LGTM: Improved method signature formatting

The reformatted method signature enhances readability by placing the return type on a separate line.


79-80: LGTM: Consistent method signature formatting

This change maintains a consistent formatting style with other method signatures.


91-92: LGTM: Enhanced readability for complex return type

Breaking the long return type onto a separate line significantly improves readability.


110-111: LGTM: Consistent method signature formatting

This formatting change completes the consistent application of the same pattern throughout the file.

lint-tasks.yaml (7)

5-5: New Variable Addition for Clang-Tidy Directory
A new variable G_LINT_CLANG_TIDY_DIR has been added to specify the directory for clang‐tidy outputs. This change helps standardise where output files are stored. Please verify that this variable is consistently used throughout the tasks that invoke clang‐tidy.


36-36: Updated Path for cpp-configs
The cpp-configs variable has been updated to point to "tools/yscope-dev-utils/exports/lint-configs/symlink-cpp-lint-configs.sh", which aligns with the new directory structure. Confirm that this path exists in the repository and is correct.


49-64: Consolidated Source File List for Clang-Format Check
The sources for the cpp-format-check task have been refactored into an anchored list (cpp_source_files), now including configuration files such as .clang-format and .clang-tidy along with directories. This strengthens consistency; however, please review that the wildcard patterns correctly capture all intended files.


67-75: Consistent Usage in cpp-format-fix Task
The cpp-format-fix task now reuses the previously defined cpp_source_files anchor, ensuring uniformity in file selection. This is a positive refactor with no apparent issues.


83-97: Enhanced clang-tidy Invocation in cpp-static-check
The task for clang‐tidy static checking has been restructured to use a dedicated utility (:utils:cpp-lint:clang-tidy-find) with flags specified as an array. The inclusion of OUTPUT_DIR linked to the newly added variable improves configuration clarity. Please confirm that the provided filename patterns and directory paths meet the project’s requirements.


134-144: Revised YAML Linting Configuration
The yml task now uses a configuration file located at "tools/yscope-dev-utils/exports/lint-configs/.yamllint.yml". This adjustment is well aligned with other linting task updates—ensure that all pertinent YAML files are addressed in the configuration.


176-191: Updated Virtual Environment Task Configurations
The venv tasks have been modified to use new utility names (e.g. :utils:misc:create-venv and :utils:checksum:compute) and updated checksum validation parameters. These changes align with the updated version of yscope-dev-utils and help ensure a more robust, parallelised linting process.

src/spider/core/Task.hpp (3)

21-33: Improved Formatting for TaskInput Constructors
The constructors in the TaskInput class have been reformatted (e.g. using initializer lists with proper line breaks and std::move) to improve code readability. These cosmetic changes do not affect functionality but do enhance maintainability.


35-38: Streamlined get_task_output Method Signature
The get_task_output method definition has been condensed into a more concise format. This change aids readability without impacting functionality.


150-153: Reformatted set_job_creation_time in ScheduleTaskMetadata
The set_job_creation_time method in the ScheduleTaskMetadata class has been reformatted into a clearer, single-line signature. This consistency in style is appreciated, as it does not alter the underlying logic.

src/spider/storage/DataStorage.hpp (4)

25-32: Uniform Reformatting for add_driver_data and add_task_data
The method signatures for add_driver_data and add_task_data have been reformatted across multiple lines for enhanced clarity. This change is purely stylistic and does not impact functionality.


37-40: Consistent Formatting for add_task_reference
The signature of add_task_reference has been adjusted to span multiple lines, promoting consistency with other function declarations.


47-52: Reformatted add_driver_reference Signature
The add_driver_reference method is now clearly presented over several lines. This cosmetic change improves readability while leaving the implementation intact.


65-78: Enhanced Clarity for Key-Value Data Methods
Both get_client_kv_data and get_task_kv_data have been reformatted, with parameter lists clearly separated from the return type. This standardisation increases readability and maintainability.

src/spider/storage/mysql/MySqlStorage.hpp (10)

78-80: Reformatted get_task in MySqlMetadataStorage
The get_task method signature has been adjusted for clarity by placing the return type and parameters on separate lines. This refactoring maintains functionality while adhering to the project's formatting standards.


87-88: Updated add_task_instance Formatting
The add_task_instance method now reflects the updated style guidelines. This clear layout aids in quick comprehension of the function signature without changing its logic.


100-102: Improved get_child_tasks Signature
The get_child_tasks method has been reformatted to ensure consistency and readability. The change is cosmetic and does not affect functionality.


106-108: Refined Heartbeat Timeout Method Declaration
The heartbeat_timeout method now follows a consistent multi-line format. Ensure that the numerical parameter timeout is being passed appropriately in all invocations.


109-113: Reorganized get_scheduler_addr Declaration
The get_scheduler_addr method signature is now more uniformly formatted. Verify that output variables for the address and port are handled correctly in the implementation.


114-116: Consistent set_scheduler_state Formatting
The method set_scheduler_state has been reformatted for better consistency with other task-related methods. No functional change; the updated style improves visual clarity.


133-134: Standardised Static Method fetch_full_task
The static method fetch_full_task has been reformatted to align with the new coding style. Although only a stylistic change, please ensure that callers of this function are updated if necessary.


151-152: Reformatted get_data in MySqlDataStorage
The method get_data now uses a clearer, multi-line format. This reformatting unambiguously separates parameters from the return type without affecting behaviour.


155-157: Updated add_task_reference in MySqlDataStorage
The reformatting of add_task_reference enhances readability by splitting the declaration across multiple lines. Functionality remains unchanged.


175-177: Consistent Formatting for add_client_kv_data
The changes to the add_client_kv_data method align with the overall formatting improvements throughout MySQL storage classes. Good consistency is maintained and no functional issues are introduced.

src/spider/storage/MetadataStorage.hpp (1)

30-138: Uniform Reformatting Across MetadataStorage Declarations
The virtual method declarations in MetadataStorage have been uniformly reformatted to separate the return types, parameters, and = 0 specifiers across distinct lines. This enhancement greatly improves readability and consistency across the interface definitions. No functional behaviour has been altered. Please ensure that the implementations adhere to these updated style conventions.

@davidlion davidlion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR seems to re-format everything (maybe because of a clang-format update or version bump). Let's separate that into its own PR so it isn't mixed with the clang-tidy update.

@sitaowang1998

Copy link
Copy Markdown
Collaborator Author

This PR seems to re-format everything (maybe because of a clang-format update or version bump). Let's separate that into its own PR so it isn't mixed with the clang-tidy update.

Unfortunately, the linting workflow will fail if code is not formatted.

@sitaowang1998
sitaowang1998 deleted the clang-tidy branch May 5, 2025 17:41
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