nuttxpr commented on PR #15088: URL: https://github.com/apache/nuttx/pull/15088#issuecomment-2527415135
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. ## NuttX PR Requirements Met? **YES**, but with some minor issues that need addressing. **Summary:** The summary explains *what* the change does (adds support for ZCU111 LEDs), and *how* it achieves this (EMIO and PL configuration). However, it lacks a clear explanation of *why* this is necessary. Is there no existing LED support on this board? Is the existing support flawed? **Impact:** The impact section is incomplete. While it mentions the ZCU111 board, it doesn't explicitly address all required points. Each point should be answered with NO/YES and an explanation if YES. For example, Documentation: YES (Updated the ZCU111 board documentation to reflect the new LED configuration). Consider the impact on other potential users of the same processor or similar FPGA configurations. **Testing:** The testing section is good. It specifies the host and target environments. The logs demonstrate the LEDs functioning. However, the "before change" logs are missing. Show the behavior before the change (presumably the LEDs didn't work). This helps demonstrate the effectiveness of the PR. **Specific Recommendations:** 1. **Strengthen the Summary:** Explain the *why*. Why are the existing methods (if any) insufficient? This provides context and justification for the change. 2. **Complete the Impact Section:** Explicitly address *all* impact points (build, hardware, documentation, security, compatibility). Even if the answer is NO, state it explicitly. For Hardware, specify the exact chip and board. For Documentation, confirm whether documentation updates are included in the PR or planned for a separate PR. 3. **Add "Before Change" Logs:** Include logs demonstrating the behavior of the LEDs *before* your changes were implemented. This highlights the problem you're solving. 4. **Consider Code Style and Comments:** Ensure the added code adheres to NuttX coding style guidelines and is well-commented. This isn't directly mentioned in the requirements but is crucial for maintainability. By addressing these points, the PR will be even stronger and easier for reviewers to evaluate. -- 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]
