nuttxpr commented on PR #15140: URL: https://github.com/apache/nuttx/pull/15140#issuecomment-2535487416
[**\[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 it is missing some crucial information. Here's a breakdown of what's good and what needs improvement: **Good:** * **Clear explanation of the *why***: You explain the reason for the changes (bug fix and supporting varying SPI frame lengths) and the context (CoreSPI, FPGA, IMU sensors). * **Description of the *what***: You outline the two distinct changes being introduced. * **Testing confirmation:** You state that testing was performed and mention the platform and different configurations. **Needs Improvement:** * **NuttX Issue/PR References:** Are there associated issues or PRs in either the NuttX or NuttX Apps repositories? If not, explicitly state "None". * **Impact - More Detail:** While you touch on the impact, be more specific: * **User Impact:** "Doesn't change existing basic functionality" is good, but confirming explicitly with "NO" is better for clarity. * **Build Impact:** Does adding this support require any build system changes (e.g., Kconfig options)? State "NO" if not. * **Hardware Impact:** Specify the affected architecture (e.g., `arch/risc-v`). Be explicit about which boards are affected (even if it's a custom board, name it). "MPFS platform" is too vague. * **Documentation Impact:** Does this require documentation updates? If so, are they included in the PR? State "YES" and describe the updates *or* "NO" if none are needed. * **Security Impact:** Explicitly state "NO" if there are no security implications. * **Compatibility Impact:** Explicitly state "NO" (if true) or detail any potential compatibility issues. * **Testing - More Detail is Crucial:** * **Build Host(s):** Provide specifics: OS version, compiler version (e.g., "Ubuntu 22.04, GCC 11.3.0"). * **Target(s):** Be extremely specific. "custom MPFS board" is insufficient. Provide details about the board and configuration (e.g., "MPFS-CustomBoard, with config XYZ enabled"). Include the RISC-V core being used. * **Testing Logs:** You *must* provide actual log snippets demonstrating the issue before the fix and the correct behavior after the fix. "Tested..." is not enough; show the evidence. Even a few lines illustrating the key changes are better than nothing. This is *essential* for reviewers to understand the problem and verify the solution. By providing this missing information, you'll make it much easier for reviewers to evaluate your PR and increase the chances of it 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
