[PR] tools/esp32s3/Config.mk: fix "printf:`\': invalid format character" [nuttx]

["tmedicci (via GitHub)" <gi...@apache.org>]

tmedicci opened a new pull request, #14438:
URL: https://github.com/apache/nuttx/pull/14438

   ## Summary
   
   * tools/esp32s3/Config.mk: fix "printf:\`': invalid format character"
   
   The error was introduced by #14393, which was intended to fix this line for MacOS. This commit makes this line dependent on the host OS to avoid any compilation errors.
   
   ## Impact
   
   Fix compile issues related to the legacy bootloader of the ESP32-S3 on both Linux and MacOS. CI didn't get the error because `CONFIG_ESP32S3_MERGE_BINS` is not tested.
   
   ## Testing
   
   Internal CI testing + ESP32-S3-DevKitC-1 v1.0 board (esp32s3-devkit:knsh + `CONFIG_ESP32S3_MERGE_BINS=y`).


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] tools/esp32s3/Config.mk: fix "printf:`\': invalid format character" [nuttx]

["xiaoxiang781216 (via GitHub)" <gi...@apache.org>]

xiaoxiang781216 merged PR #14438:
URL: https://github.com/apache/nuttx/pull/14438


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] tools/esp32s3/Config.mk: fix "printf:`\': invalid format character" [nuttx]

["nuttxpr (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org