nuttxpr commented on PR #14336: URL: https://github.com/apache/nuttx/pull/14336#issuecomment-2414540298
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR description you provided is quite good, but here's a breakdown of how well it aligns with NuttX requirements and some suggestions for improvement: **Strengths:** * **Clear Summary:** You clearly state the need (no existing driver) and the solution (adding a new driver based on STM32L4). The mention of dual bank and page size differences is crucial. * **Impact Section:** You correctly identify the specific impact area (STM32G4 chips within the STM32 category). * **Testing Description:** You provide a reasonable overview of your testing approach, including: * Target hardware (Nucleo-G474RE) * Functional testing of driver functions * Integration testing with `/dev/config` and mtd * Regression testing by building with other architectures. **Areas for Improvement (to fully meet NuttX requirements):** * **Formalized Sections:** While you address the content, explicitly use the headings "Summary," "Impact," and "Testing" as in the template. * **Concise Summary:** Trim down the Summary to its essence. For example: ``` **Summary:** * Adds a flash driver for the STM32G4 series, addressing the lack of support. * Based on the STM32L4 driver, but handles dual bank and dynamic page size differences in category 3 devices. ``` * **Detailed Impact Breakdown:** While you identify the impacted area, go through each sub-bullet in the template and explicitly state "NO" or provide a brief explanation if "YES": ``` **Impact:** * Is new feature added? **YES** (STM32G4 flash driver support) * Is existing feature changed? **NO** * Impact on user ... **NO** * Impact on build ... **NO** * Impact on hardware... **YES** (Only affects STM32G4 chips within the STM32 architecture) * Impact on documentation... **YES** (Driver documentation should be added) * ... (Continue for all impact points) ``` * **Testing Logs:** The template explicitly asks for "Testing logs before change" and "Testing logs after change." While full logs might be lengthy, include snippets that demonstrate key functionality before and after your change. This provides concrete evidence of the driver's correct operation. * **Issue References:** If this PR addresses a specific issue in the NuttX or NuttX Apps repositories, include the issue number(s) under the Summary section for better tracking. * **Contributing Guidelines Link:** The provided link to the Contributing Guidelines seems to be cut off. Ensure the link is correct. **In Summary** Your PR description is on the right track. By incorporating the suggested improvements, particularly the explicit use of the template structure, detailed impact breakdown, and inclusion of testing logs, you'll create a PR that fully aligns with NuttX's requirements, making it easier for reviewers to understand and accept your contribution. -- 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
