nuttxpr commented on PR #14821: URL: https://github.com/apache/nuttx/pull/14821#issuecomment-2480613716
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR appears to mostly meet the NuttX requirements, but is missing some crucial details. Here's a breakdown: **Strengths:** * **Summary:** Provides a decent overview of the changes made. Listing multiple distinct changes like this is helpful. * **Impact:** While "minor" is concise, it lacks specificity. The impact section needs to address *all* the listed impact points (user, build, hardware, documentation, security, compatibility). While the impact *might* be "no" for most of these, it needs to be explicitly stated. **Weaknesses:** * **Summary:** Missing issue references if any of these changes relate to reported issues. While not strictly required if no issue exists, it's good practice to create one for non-trivial changes. * **Impact:** Severely lacking detail. Even if the answer is "NO" for each item, it needs to be explicitly stated (e.g., "Impact on user: NO"). This PR touches scheduling, sensors, and the simulator; at least *some* of the impact categories are likely affected. For example, replacing `nuttx_mode_t` with `int` *could* have compatibility implications. These need to be explained. * **Testing:** "ci" is insufficient. While CI testing is important, the requirements explicitly state "verified on local setup." The PR needs to specify the host and target details, as well as provide *actual* log snippets demonstrating the fix. Just saying "ci" doesn't prove the change works as intended. **Recommendation:** The PR author needs to expand on the Impact and Testing sections significantly. They need to explicitly address each point in the Impact section, even if the answer is "NO." The Testing section needs real host/target details and log output demonstrating the before/after behavior. The Summary section could also benefit from linked issue references if any exist. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
