-
Notifications
You must be signed in to change notification settings - Fork 6
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
style: Improve code style by putting private after public #17
Conversation
WalkthroughThe pull request includes modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Data
participant TaskInput
participant TaskOutput
participant TaskGraph
User->>Data: Access member variables
Data->>User: Return values
User->>TaskInput: Access member variables
TaskInput->>User: Return values
User->>TaskOutput: Access member variables
TaskOutput->>User: Return values
User->>TaskGraph: Access tasks and dependencies
TaskGraph->>User: Return tasks and dependencies
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/spider/core/Task.hpp (1)
41-45
: Add explicit private section marker and fix indentationThe private member variables should be properly grouped under an explicit
private:
section marker to improve readability and maintain consistency with the class structure pattern used elsewhere in the codebase.Apply this diff to improve the structure:
- std::optional<std::tuple<boost::uuids::uuid, std::uint8_t>> m_task_output; - std::optional<std::string> m_value; - std::optional<boost::uuids::uuid> m_data_id; - std::string m_type; +private: + std::optional<std::tuple<boost::uuids::uuid, std::uint8_t>> m_task_output; + std::optional<std::string> m_value; + std::optional<boost::uuids::uuid> m_data_id; + std::string m_type;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/spider/core/Data.hpp
(2 hunks)src/spider/core/Task.hpp
(2 hunks)src/spider/core/TaskGraph.hpp
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/spider/core/Data.hpp
- src/spider/core/TaskGraph.hpp
🔇 Additional comments (2)
src/spider/core/Task.hpp (2)
64-68
: LGTM! Well-structured member organization
The private member variables are properly organized with correct access control marker and consistent indentation. The empty line before the private section provides good visual separation.
41-45
: Consider standardizing member organization across all classes
While TaskOutput follows the new style perfectly, there's inconsistency in how private members are organized across different classes in this file. Consider:
- Applying the same style (with explicit private marker and spacing) to TaskInput
- Updating the Task class to follow the same pattern for consistency
Let's check if this style is being consistently applied across the codebase:
Also applies to: 64-68
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/spider/core/Task.hpp (1)
120-129
: LGTM! Private members well-organized with proper separation.The private section follows good C++ practices:
- Clear separation from public interface with an empty line
- Logical grouping of related members (identifiers, state, configuration, collections)
Consider adding comments to group related private members for better maintainability, especially since this class has more members than the others:
private: + // Identification boost::uuids::uuid m_id; std::string m_function_name; + // State and metadata TaskState m_state = TaskState::Pending; TaskCreatorType m_creator_type; boost::uuids::uuid m_creator_id; float m_timeout = 0; + // Task graph connections std::vector<TaskInput> m_inputs; std::vector<TaskOutput> m_outputs;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/spider/core/Task.hpp
(3 hunks)
🔇 Additional comments (2)
src/spider/core/Task.hpp (2)
41-45
: LGTM! Private members properly placed after public interface.
The placement of private members after the public interface follows the Google C++ Style Guide recommendations, improving readability by presenting the public API first.
64-68
: LGTM! Well-structured private section with proper separation.
The private section is properly separated from the public interface with an empty line, and the member variables are logically grouped together.
Description
Improve code style by putting private after public in class definition.
Validation performed
Summary by CodeRabbit
Data
,TaskInput
,TaskOutput
, andTaskGraph
classes for improved structure.TaskInput
andTask
classes from private to public.TaskOutput
andTaskGraph
classes without altering functionality.