You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "zhhyu7 (via GitHub)" <gi...@apache.org> on 2024/11/04 11:40:52 UTC

[PR] netinet/if_ether.h: move #define ETH_XXX into if_ether.h [nuttx]

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

   
   ## Summary
   consistent with other operating systems
   
   ## Impact
   
   ## Testing
   qemu:x86_64
   
   


-- 
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] netinet/if_ether.h: move #define ETH_XXX into if_ether.h [nuttx]

Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #14636:
URL: https://github.com/apache/nuttx/pull/14636#issuecomment-2454504907

   [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   No, this PR description does not meet the NuttX requirements.  It's severely lacking in critical information.
   
   Here's why:
   
   * **Summary:**  "consistent with other operating systems" is vague and unhelpful.  It doesn't explain *what* change was made, *why* it was necessary, *how* it works, or which part of the code is affected.  Issue/PR references are also missing.
   * **Impact:** This section is completely empty.  All impact areas need to be addressed with "YES" or "NO" and a description if "YES."
   * **Testing:**  While it mentions qemu:x86_64, this is insufficient.  It needs to specify the host OS, compiler, specific NuttX configuration, and, crucially, *what tests were run*.  The "Testing logs before change" and "Testing logs after change" sections are also empty.  Without these logs, there's no way to verify the claimed improvement.
   
   **To make this PR acceptable, the author needs to:**
   
   1. **Provide a detailed summary:** Explain the specific change, its rationale, its implementation, and link any related issues.
   2. **Complete the Impact section:** Address all impact categories with YES/NO and explanations where necessary.
   3. **Provide complete testing information:** Detail the host and target environments, the specific NuttX configuration tested, the tests performed, and *most importantly*, the logs from before and after the change.  Without logs, the testing claim is unverifiable.
   


-- 
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] netinet/if_ether.h: move #define ETH_XXX into if_ether.h [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #14636:
URL: https://github.com/apache/nuttx/pull/14636


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