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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   This PR does **not** fully meet the NuttX requirements.  While it provides a 
summary of *what* the change does, it lacks crucial information.
   
   Here's a breakdown of what's missing:
   
   **Summary:**
   
   * **Missing "Why":**  The summary explains the *how* but not the *why*.  Why 
is per-thread MTE control necessary? What problem does it solve? Is there a bug 
being fixed? Is this a performance optimization?
   * **Missing Scope:** What specific files/modules are affected?  Saying "the 
method is the same as..." isn't descriptive enough.  Name the specific 
functions or files modified.
   * **Missing Issue References:**  Are there any related NuttX or NuttX-apps 
issues that prompted this change? If so, link them.
   
   **Impact:**
   
   * **Completely Empty:** This section is critical.  The maintainer needs to 
understand the ramifications of merging this change.  Address *all* the points 
(user impact, build impact, hardware impact, documentation, security, 
compatibility).  Even if the answer is "NO", explicitly state it.  For example: 
"Impact on user: NO"
   * **Security implications are especially important here** given that this 
relates to MTE (Memory Tagging Extension), a security feature.  What are the 
potential security benefits or drawbacks of this change?
   
   **Testing:**
   
   * **Completely Empty:**  This section is also crucial. Without evidence of 
testing, the PR is unlikely to be merged.  Provide specific details:
       * **Build Host:**  e.g., "Linux (Ubuntu 22.04), x86_64, GCC 11.2.0"
       * **Target:** e.g., "qemu-rv64:nsh"
       * **Logs:**  Include *actual* logs showing the behavior before and after 
the change.  Demonstrate that the change works as intended and doesn't 
introduce regressions.  If the logs are too long, consider providing a link to 
a gist or pastebin.
   
   **In short, the PR needs significant improvements before it can be 
considered for merging.**  It needs to be more thorough and provide the 
necessary information for reviewers to evaluate the change effectively.  Follow 
the template more closely and fill in all the required sections.
   


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