nuttxpr commented on PR #13641: URL: https://github.com/apache/nuttx/pull/13641#issuecomment-2376619578
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary is good, but the rest needs some work to meet the NuttX PR requirements. Here's a breakdown: **What's Good:** * **Summary:** Clearly explains the *what* (switching from wget to cURL, using shallow clones) and the *where* (specific toolchains and kconfig_frontends). **What Needs Improvement:** * **Summary:** * **Why:** It's missing the *why* behind these changes. Are there benefits to using cURL over wget? Is it for consistency, efficiency, or to resolve specific issues? * **How (details):** Provide a brief explanation of how the switch to cURL and the use of shallow clones will be implemented. * **Issue References:** Link any related NuttX issues this PR addresses. * **Impact:** * **Build:** Even though the process remains the same, mention if there are any expected changes in build times (e.g., faster downloads due to cURL or shallow clones). * **Hardware:** While unlikely, confirm if this has any impact on specific architectures or boards. * **Compatibility:** Address potential compatibility concerns. Does switching to cURL introduce any version dependencies? * **Other:** Are there any security considerations with using cURL vs. wget? * **Be Specific:** Avoid generic "NO" answers. Even if there's no impact, briefly explain why (e.g., "Impact on user: No, as this change only affects internal build scripts."). * **Testing:** * **Insufficient:** "CI" is not enough detail. * **Host/Target:** Specify the build hosts (OS, CPU, compiler) and the targets (architecture, board, config) you used for testing. * **Logs (Optional but Recommended):** While not always mandatory, including snippets of relevant log output (especially if it highlights the changes) can be very helpful for reviewers. **Example Improvements:** **Impact:** * **Build:** "Build process remains the same. The switch to cURL and shallow clones is expected to slightly reduce download times during the build." * **Hardware:** "No impact on hardware. These changes affect only the build scripts and toolchain downloads." * **Compatibility:** "This change may introduce a minimum cURL version requirement. The specific version will be determined during implementation." **Testing:** * "I confirm that the changes were verified on the following setups: * **Build Host:** Ubuntu 20.04, x86_64, GCC 9.4.0 * **Target:** * sim:nsh_stm32f4discovery * arm:stm32f4discovery:nsh" -- 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]
