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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   This PR appears to **partially meet** the NuttX requirements. While it 
provides some information, crucial details are missing, making it difficult to 
fully assess the changes.
   
   **Missing Information/Areas for Improvement:**
   
   * **Summary:**  While the *what* and *how* are somewhat explained, the *why* 
is weak.  What specific errors were encountered due to inconsistent register 
storage? Providing concrete examples of the problems this change solves would 
significantly strengthen the justification.
   * **Impact:** The description is overly concise and lacks detail.  It 
mentions "context switch logic," but doesn't explain *how* the context switch 
logic is affected. Be specific about potential side-effects. Address all the 
impact points individually (user, build, hardware, documentation, security, 
compatibility).  Even if the answer is "NO," explicitly state it for each item.
   * **Testing:**  The testing description is inadequate.  Simply stating that 
it was compiled and run on QEMU doesn't demonstrate that the changes work *as 
intended.*  
       *  What specific tests were run to verify the fix?  
       *  What were the *expected* results of those tests both before and after 
the change?
       *  The provided logs are empty.  Include actual log output demonstrating 
the issue before the change and the corrected behavior after the change.  Show 
how the changes addressed the original problem.
       *  Consider adding a test case to the NuttX test suite to prevent 
regressions.
   
   **Example of improved testing section:**
   
   **Testing**
   **Build Host:**
   * OS: Ubuntu 20.04
   * CPU: x86_64 
   * Compiler: GCC 9.4.0
   
   **Target:**
   * Arch: sim (QEMU)
   * Board/Config: qemu-armv8a:nsh_smp
   
   **Test Case:** Created a test application that repeatedly triggers a 
specific interrupt (e.g., timer interrupt) and checks the consistency of 
register values before and after the interrupt handler.
   
   **Testing logs before change:**
   
   ```
   <demonstrate inconsistent register values, e.g., incorrect value restored 
after interrupt>
   ERROR: Register X12 expected 0x12345678, actual 0x87654321
   ```
   
   **Testing logs after change:**
   
   ```
   <demonstrate correct and consistent register values after the interrupt>
   INFO: Register X12 expected 0x12345678, actual 0x12345678
   All register checks passed.
   ```
   
   
   By providing more specific information in these sections, the PR will be 
much easier to review and understand, increasing the likelihood of acceptance.
   


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