nuttxpr commented on PR #14438: URL: https://github.com/apache/nuttx/pull/14438#issuecomment-2427185471
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary and impact sections are well-written and informative. They clearly explain the issue, the proposed solution, and the impact of the change. The testing section, however, could be improved. Here's a breakdown and suggestion: **Strengths:** * **Clear Problem Statement:** The summary clearly identifies the bug ("printf:\`': invalid format character") and its root cause (a previous PR intended to fix a MacOS issue). * **Solution Explained:** The summary also outlines the solution: making the problematic line OS-dependent. * **Impact Clearly Defined:** The impact section accurately describes which users and systems are affected by the bug fix (users building for ESP32-S3, specifically with the legacy bootloader and the `CONFIG_ESP32S3_MERGE_BINS` option). **Area for Improvement:** * **Insufficient Testing Details:** While the testing section mentions CI testing and a specific board, it lacks crucial information: * **No Build Host Details:** Specify the OS, CPU, and compiler used for local testing. * **Missing Logs:** The "Testing logs before change" and "Testing logs after change" sections are empty. Provide actual log snippets demonstrating the error before the fix and its resolution afterward. **Recommendation:** To strengthen the testing section, include the following: 1. **Build Host Information:** Add details about the local setup used for verification (e.g., "Build Host: Linux, x86_64, GCC 12.2.0"). 2. **Relevant Log Excerpts:** Provide concise snippets from the build logs. Highlight the error message encountered before the change and demonstrate its absence after applying the fix. By providing these details, you enhance the PR's credibility, making it easier for reviewers to assess the effectiveness of the proposed solution. -- 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]
