nuttxpr commented on PR #15378: URL: https://github.com/apache/nuttx/pull/15378#issuecomment-2564743915
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. While it provides a summary and mentions testing, it lacks crucial details. Here's a breakdown of the missing information: * **Summary:** While it explains the "what," it lacks the "why." Why is this migration warning necessary? What problem does it solve? What is the migration about? The summary should clearly state the motivation behind the change. It also lacks references to related issues, if any. * **Impact:** The impact section is too vague. While it mentions documentation changes, it doesn't specify *which* documentation files are modified. All other impact categories are simply implied as "NO" but should be explicitly stated as such for clarity. For example: * Impact on user: NO * Impact on build: NO * ...and so on for all categories. * **Testing:** "Validated locally with `make autobuild`" is insufficient. Provide specific details: * **Build Host(s):** Specify your OS, CPU architecture, and compiler version. * **Target(s):** Which architecture(s) and board/config combinations did you test on? Just stating `make autobuild` doesn't say *what* was built. * **Logs:** The requirement clearly asks for "Testing logs before change" and "Testing logs after change." These are missing entirely. Even if there's no functional change, showing a successful build before and after the documentation change is good practice. **Example of how to improve the PR description:** ## Summary This PR adds a migration warning to the update release system and the ELF programs documentation pages. This is necessary to provide users with a clear reference to the official wiki page outlining the migration process for [briefly explain what is being migrated, e.g., the new configuration system]. This addresses the potential for confusion and errors during the migration, ensuring users have easy access to the definitive migration guide. (Related Issue: # if applicable) ## Impact * Is new feature added? NO * Is existing feature changed? NO * Impact on user: NO (Only documentation is changed to provide additional guidance). * Impact on build: NO * Impact on hardware: NO * Impact on documentation: YES (Added a warning section to `docs/update_release_system.txt` and `docs/elf_programs.txt` referencing the wiki page at [link to wiki page]). * Impact on security: NO * Impact on compatibility: NO * Anything else to consider: NO ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 * Target(s): sim:nsh_romfsim Testing logs before change: ``` make autobuild CONFIG_SIM=y CONFIG_NSH_ROMFSIM=y ... (relevant build output showing successful build) ``` Testing logs after change: ``` make autobuild CONFIG_SIM=y CONFIG_NSH_ROMFSIM=y ... (relevant build output showing successful build) ``` -- 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
