nuttxpr commented on PR #15442:
URL: https://github.com/apache/nuttx/pull/15442#issuecomment-2574408586

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   No, this PR description does not adequately meet the NuttX requirements.  
Here's why and how to fix it:
   
   **Missing Information/Insufficient Detail:**
   
   * **Summary:**  "Causing c5471evm/httpd ci break" is a symptom, not a 
summary.  Explain *what* change is being made to fix the break.  What was the 
root cause of the CI break?  Be explicit.  Mentioning the `irq.h` path is a 
start but doesn't explain the *change* itself.  Is a function being added, 
removed, or modified?
   * **Impact - User:** Does this fix impact how the user interacts with the 
`c5471evm/httpd` functionality?  Likely not, but explicitly stating "NO" is 
better.
   * **Impact - Build:** Probably "NO," but confirm. If the include paths were 
modified, you might need to mention that.
   * **Impact - Hardware:**  Specify "c5471evm" explicitly under hardware 
impact. While arm32 is mentioned, being specific about the board is crucial.
   * **Impact - Documentation:**  If the change affects how something works, 
even internally, consider if documentation needs updates (likely "NO" here, but 
state it).
   * **Impact - Security, Compatibility, Other:** Explicitly state "NO" for 
each if not applicable.
   * **Testing:** "CI-test" is insufficient. Provide specific CI run links. 
"Local build" needs details – which OS, compiler, etc. were used? Critically, 
you **must** provide *actual* log snippets showing the error *before* the fix 
and the successful operation *after* the fix. Empty code blocks as provided 
don't demonstrate anything.
   
   
   **How to Improve:**
   
   ```
   ## Summary
   
   This PR fixes a CI break for the c5471evm/httpd configuration introduced by 
a previous change (mention the commit hash or PR number if possible). The 
previous change inadvertently caused the default irq.h path to be incorrect, 
resulting in a missing `up_getusrsp` symbol during compilation.  This PR 
corrects the irq.h include path to ensure the correct definition is available.
   
   Related NuttX Issue: #15425
   
   ## Impact
   
   * Is new feature added? NO
   * Is existing feature changed? NO (Bug fix restoring previous functionality)
   * Impact on user: NO
   * Impact on build: NO
   * Impact on hardware: YES (c5471evm)
   * Impact on documentation: NO
   * 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: Linux (Ubuntu 22.04), x86_64, GCC 11.2.0
   * Target: arm, c5471evm:default
   
   Testing logs before change:
   
   ```
   /path/to/build/directory/httpd: undefined reference to `up_getusrsp'
   collect2: error: ld returned 1 exit status
   make: *** [/path/to/build/directory/httpd] Error 1
   ```
   
   Testing logs after change:
   ```
   Linking /path/to/build/directory/httpd
   ... (other build output)
   Successfully built target httpd
   ```
   
   CI test results:
   * [Link to successful CI run]
   ```
   
   
   
   By providing specifics and addressing all the required points, the PR will 
be much easier to review and merge.  Remember to replace the placeholders like 
"[Link to successful CI run]" with the actual information.
   


-- 
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]

Reply via email to