nuttxpr commented on PR #15822: URL: https://github.com/apache/nuttx/pull/15822#issuecomment-2653305035
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the basic NuttX requirements, but is missing crucial details. While it addresses the summary and testing sections at a high level, it lacks the specificity required for a proper review. Here's a breakdown of what's missing and how to improve it: **Summary:** * **Insufficient Detail:** "Wrong evaluation of `dev->d_lltype`" is too vague. Explain *how* the evaluation was wrong and *what* the correct evaluation should be. Provide specific code examples if possible. * **Missing Context:** What functional part of the code is affected? (e.g., network stack, 6lowpan driver, etc.) Be precise. **Impact:** While it mentions enabling TCP and ICMP over 6lowpan, it needs to be more thorough by explicitly answering *all* the impact questions with "YES" or "NO" and providing details where necessary. For example: * **Is new feature added?** Arguably no, as this is fixing a bug, enabling existing functionality. * **Is existing feature changed?** YES (6lowpan now supports TCP and ICMP). * **Impact on user:** YES (Users can now use TCP and ICMP over 6lowpan) * All other impact categories should be addressed with a YES/NO and explanation. Even if the answer is NO, explicitly state it. **Testing:** * **Insufficient Detail:** "Tested on ESP32 using ESP-NOW" is not enough. Specify the ESP32 board, the NuttX configuration used, and the ESP-NOW configuration. * **Missing Logs:** The "Testing logs before change" and "Testing logs after change" sections are empty. Include actual logs demonstrating the issue before the change and the successful operation after the change. Show how you verified TCP and ICMP functionality. Consider using tools like `tcpdump` or `ping6`. * **Build Host Details Missing:** Provide details about your build host environment as requested. **Overall:** The PR needs significant improvement in terms of detail and completeness. The current information provides a high-level overview but lacks the necessary specifics for reviewers to understand the change, its impact, and its validation. Providing the requested details will significantly improve the review process and increase the likelihood of the PR being accepted. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
