-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[Add Comments] components:libc:cplusplus:os/utest:Some comments have been initially … #10870
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
base: master
Are you sure you want to change the base?
Conversation
…added. Some comments have been initially added. components/libc/cplusplus/os/cxx_Semaphore.cpp components/libc/cplusplus/os/cxx_Thread.cpp components/libc/cplusplus/utest/tc_atomic.cpp components/libc/cplusplus/utest/tc_smartptr.cpp components/libc/cplusplus/utest/tc_thread.cpp Signed-off-by:Liu Chengtao<2739960959@qq.com>
|
|
|
👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。 🛠 操作步骤 | Steps
完成后,提交将自动更新至 如有问题欢迎联系我们,再次感谢您的贡献!💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
🏷️ Tag: components_libcReviewers: GorrayLi mysterywolf Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-11-13 18:30 CST)
📝 Review Instructions
|
|
/check-cla |
title:Everything is based on remote repositories author:Liu Chengtao<2739960959@qq.com>
| UTEST_UNIT_RUN(test_thread); | ||
| } | ||
|
|
||
| // Export the test case with initialization and cleanup functions and a timeout of 10 ticks. |
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.
代码注释的风格统一下,需要使用/**/
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.
Pull Request Overview
This PR adds English comments and documentation to C++ wrapper classes and unit test files in the RT-Thread libc/cplusplus component. The changes aim to improve code readability and documentation for beginners, though several issues need to be addressed to align with RT-Thread documentation standards.
Key Changes:
- Added Doxygen-style comments to constructors, methods, and test functions in C++ OS wrapper classes (Thread, Semaphore)
- Added inline and function-level comments to unit test files (tc_thread, tc_smartptr, tc_atomic)
- Minor code formatting adjustments (whitespace alignment in tc_atomic.cpp, constructor initializer list formatting in cxx_Thread.cpp)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
| components/libc/cplusplus/utest/tc_thread.cpp | Added Doxygen comments to test functions and inline comments explaining test logic |
| components/libc/cplusplus/utest/tc_smartptr.cpp | Added Doxygen comments to initialization and cleanup functions |
| components/libc/cplusplus/utest/tc_atomic.cpp | Added simple comments to init/cleanup functions and adjusted variable alignment formatting |
| components/libc/cplusplus/os/cxx_Thread.cpp | Added Doxygen comments to Thread class methods and constructors, modified constructor initializer list formatting |
| components/libc/cplusplus/os/cxx_Semaphore.cpp | Added Doxygen comments to Semaphore class methods and inline explanatory comments |
| * Join the thread with a timeout. | ||
| * @param millisec Timeout in milliseconds. | ||
| * @return Status code indicating the execution status. |
Copilot
AI
Nov 13, 2025
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.
Incomplete Doxygen Comment / 不完整的 Doxygen 注释
English: The comment is missing the @brief tag and should document both the parameter and return value.
中文:注释缺少 @brief 标签,并且应该记录参数和返回值。
Suggested format / 建议格式:
/**
* @brief Join the thread with a timeout.
* @param millisec Timeout in milliseconds
* @return RT_EOK if the thread completed within the timeout, error code otherwise
*/| * Join the thread with a timeout. | |
| * @param millisec Timeout in milliseconds. | |
| * @return Status code indicating the execution status. | |
| * @brief Join the thread with a timeout. | |
| * @param millisec Timeout in milliseconds. | |
| * @return RT_EOK if the thread completed within the timeout, error code otherwise. |
| for (int i = 0; i < 100; ++i) | ||
| { | ||
| ++count; | ||
| ++count; // Increment the count 100 times. |
Copilot
AI
Nov 13, 2025
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.
[nitpick] Redundant Comment / 冗余注释
English: The comment "Increment the count 100 times" is redundant since the for loop already clearly shows i < 100. This is an example of commenting "what" instead of "why." Remove this comment as it adds no value.
中文:注释"Increment the count 100 times"是冗余的,因为 for 循环已经清楚地显示了 i < 100。这是注释"是什么"而不是"为什么"的例子。删除此注释,因为它没有增加任何价值。
| ++count; // Increment the count 100 times. | |
| ++count; |
| // Create and run a thread executing the lambda function. | ||
| std::thread t1(func); | ||
| t1.join(); | ||
| t1.join(); // Wait for the thread to finish execution. | ||
|
|
||
| // Verify if the count is as expected after the first thread execution. | ||
| if (count != 100) | ||
| { | ||
| uassert_false(1); | ||
| uassert_false(1); // Assert failure if count is not 100. | ||
| } | ||
|
|
||
| // Create and run another thread executing the same lambda function. | ||
| std::thread t2(func); | ||
| t2.join(); | ||
| t2.join(); // Wait for the second thread to finish execution. | ||
|
|
||
| // Verify if the count is as expected after the second thread execution. | ||
| if (count != 200) | ||
| { | ||
| uassert_false(1); | ||
| uassert_false(1); // Assert failure if count is not 200. |
Copilot
AI
Nov 13, 2025
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.
[nitpick] Redundant Inline Comments / 冗余的行内注释
English: The inline comments on lines 33, 38, 43, and 48 simply restate what the code does (join, assert, join, assert). These are obvious from reading the code. According to RT-Thread documentation standards, avoid comments that merely describe what the code does when it's self-evident. Better to have a single comment explaining the test logic if needed.
中文:第 33、38、43 和 48 行的行内注释仅重述了代码的功能(join、assert、join、assert)。从代码阅读中这些是显而易见的。根据 RT-Thread 文档标准,避免仅描述代码功能的注释,当代码本身是不言自明的时候。如果需要,最好使用单个注释来解释测试逻辑。
| const char *name) : | ||
| _entry(RT_NULL), _param(RT_NULL), started(false) |
Copilot
AI
Nov 13, 2025
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.
Code Formatting Issue / 代码格式问题
English: The brace placement doesn't follow RT-Thread coding style. According to RT-Thread coding standards, braces should be on separate lines, not on the same line as the colon in constructor initializer lists. The original formatting was more aligned with the standard.
中文:大括号位置不符合 RT-Thread 编码风格。根据 RT-Thread 编码标准,大括号应独占一行,而不是与构造函数初始化列表中的冒号在同一行。原始格式更符合标准。
Original format (preferred) / 原格式(推荐):
const char *name)
: _entry(RT_NULL), _param(RT_NULL), started(false)
{Current format (non-standard) / 当前格式(不标准):
const char *name) :
_entry(RT_NULL), _param(RT_NULL), started(false)
{| using namespace rtthread; | ||
|
|
||
| /** | ||
| * @brief Thread class constructor with parameters for stack size, priority, tick, and name. |
Copilot
AI
Nov 13, 2025
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.
[nitpick] Incomplete Doxygen Comment / 不完整的 Doxygen 注释
English: The Doxygen comment should document all parameters and follow complete documentation format. Add @param tags for each parameter to make the documentation complete and useful.
中文:Doxygen 注释应记录所有参数并遵循完整的文档格式。为每个参数添加 @param 标签,使文档完整且有用。
Suggested format / 建议格式:
/**
* @brief Thread class constructor with parameters for stack size, priority, tick, and name.
* @param stack_size Stack size in bytes
* @param priority Thread priority
* @param tick Time slice in ticks
* @param name Thread name
*/| * @brief Thread class constructor with parameters for stack size, priority, tick, and name. | |
| * @brief Thread class constructor with parameters for stack size, priority, tick, and name. | |
| * @param stack_size Stack size in bytes | |
| * @param priority Thread priority | |
| * @param tick Time slice in ticks | |
| * @param name Thread name |
| /** | ||
| * @brief Test case cleanup function. | ||
| */ |
Copilot
AI
Nov 13, 2025
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.
[nitpick] Incomplete Doxygen Comment / 不完整的 Doxygen 注释
English: The Doxygen comment should document the return value. Add @return RT_EOK on success for consistency and completeness.
中文:Doxygen 注释应记录返回值。添加 @return RT_EOK on success 以保持一致性和完整性。
| UTEST_UNIT_RUN(test_thread); | ||
| } | ||
|
|
||
| // Export the test case with initialization and cleanup functions and a timeout of 10 ticks. |
Copilot
AI
Nov 13, 2025
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.
Comment Style Inconsistency / 注释风格不一致
English: This comment uses // style while other function documentation in the file uses /** */ Doxygen style. For consistency across the file, use the Doxygen format with @brief tag like the other functions.
中文:此注释使用 // 风格,而文件中的其他函数文档使用 /** */ Doxygen 风格。为了整个文件的一致性,请使用带有 @brief 标签的 Doxygen 格式,就像其他函数一样。
Suggested format / 建议格式:
/**
* @brief Export the test case with initialization and cleanup functions and a timeout of 10 ticks.
*/| // Export the test case with initialization and cleanup functions and a timeout of 10 ticks. | |
| /** | |
| * @brief Export the test case with initialization and cleanup functions and a timeout of 10 ticks. | |
| */ |
| // Test case initialization function. | ||
| static rt_err_t utest_tc_init(void) | ||
| { | ||
| return RT_EOK; | ||
| } | ||
|
|
||
| // Test case cleanup function. |
Copilot
AI
Nov 13, 2025
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.
Comment Style Inconsistency / 注释风格不一致
English: This comment uses // style while the file contains Doxygen-style comments elsewhere. For consistency, use the Doxygen format with /** */ and @brief tag.
中文:此注释使用 // 风格,而文件中其他地方包含 Doxygen 风格的注释。为了一致性,请使用带有 /** */ 和 @brief 标签的 Doxygen 格式。
Suggested format / 建议格式:
/**
* @brief Test case initialization function.
* @return RT_EOK on success
*/| // Test case initialization function. | |
| static rt_err_t utest_tc_init(void) | |
| { | |
| return RT_EOK; | |
| } | |
| // Test case cleanup function. | |
| /** | |
| * @brief Test case initialization function. | |
| * @return RT_EOK on success | |
| */ | |
| static rt_err_t utest_tc_init(void) | |
| { | |
| return RT_EOK; | |
| } | |
| /** | |
| * @brief Test case cleanup function. | |
| * @return RT_EOK on success | |
| */ |
| const char *name) | ||
| : _entry(entry), _param(p), started(false) | ||
| const char *name) : | ||
| _entry(entry), _param(p), started(false) |
Copilot
AI
Nov 13, 2025
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.
Code Formatting Issue / 代码格式问题
English: The brace placement doesn't follow RT-Thread coding style. According to RT-Thread coding standards, braces should be on separate lines, not on the same line as the colon in constructor initializer lists.
中文:大括号位置不符合 RT-Thread 编码风格。根据 RT-Thread 编码标准,大括号应独占一行,而不是与构造函数初始化列表中的冒号在同一行。
Original format (preferred) / 原格式(推荐):
const char *name)
: _entry(entry), _param(p), started(false)
{| _entry(entry), _param(p), started(false) | |
| _entry(entry), _param(p), started(false) |
|
|
||
| /** | ||
| * @brief Start the thread execution. | ||
| * @return Boolean indicating if the thread was successfully started. |
Copilot
AI
Nov 13, 2025
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.
[nitpick] Incomplete Doxygen Comment / 不完整的 Doxygen 注释
English: The comment should include @return documentation to describe the return value properly.
中文:注释应包含 @return 文档以正确描述返回值。
Suggested / 建议:
/**
* @brief Start the thread execution.
* @return true if the thread was successfully started, false otherwise
*/| * @return Boolean indicating if the thread was successfully started. | |
| * @return true if the thread was successfully started, false otherwise |
…pecification Modify the os:cxx_Semaphore.cpp and cxx_Thread.cpp to comply with the rtthread specification. Change the comment symbol in utest:tc_thread.cpp from '//' to '/**/' to comply with rtthread specifications.For utest:tc_atomic.cpp and tc_smartptr.cpp,select the original respository files without modification. signed-off-by: Liu Chengtao<2739960959@qq.com>
…added.
Some comments have been initially added.
components/libc/cplusplus/os/cxx_Semaphore.cpp
components/libc/cplusplus/os/cxx_Thread.cpp
components/libc/cplusplus/utest/tc_atomic.cpp
components/libc/cplusplus/utest/tc_smartptr.cpp
components/libc/cplusplus/utest/tc_thread.cpp
Signed-off-by:Liu Chengtao2739960959@qq.com
拉取/合并请求描述:(PR description)
[
Provide comments for some files in the components/libc/cplusplus/os or components/libc/cplusplus/utest folder to complete the beginner task.
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up