Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

local_socket: corrent send/recv return value after shutdown #14103

Merged
merged 1 commit into from
Oct 12, 2024

Conversation

GUIDINGLI
Copy link
Contributor

Summary

local_socket: corrent send/recv return value after shutdown

Impact

local socket

Testing

sim

@github-actions github-actions bot added Area: Networking Effects networking subsystem Size: S The size of the change in this PR is small labels Oct 11, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 11, 2024

[Experimental Bot, please feedback here]

No, this PR does not meet the NuttX requirements.

Here's why:

  • Insufficient Summary:
    • The summary is too brief. It doesn't explain:
      • Why the change is necessary (what problem does it fix?)
      • What functional part of the local socket code is affected.
      • How the change addresses the problem (what was the incorrect behavior, and how is it corrected?)
  • Incomplete Impact Assessment:
    • The "Impact" section is overly simplistic. It should clearly state "YES" or "NO" for each impact category and provide specific details when applicable. For example:
      • Is new feature added? NO
      • Is existing feature changed? YES (local socket send/recv behavior after shutdown)
      • Impact on user (will user need to adapt to change)? This needs a clear YES/NO and explanation. If the return values from send/recv are different, users might need to adjust their code.
      • Impact on build, hardware, documentation, security, compatibility - All listed as "local socket" which is not helpful. These should be YES/NO with explanations.
  • Missing Testing Details:
    • The testing section is extremely vague:
      • Build Host(s): Be specific about your development environment (OS, CPU architecture, compiler version).
      • Target(s): "sim" is not enough information. Which simulator? Which architecture? Which NuttX configuration?
      • Testing logs: You haven't provided any logs showing the behavior before and after your changes. This is crucial evidence that your fix works as intended.

To improve this PR:

  1. Expand the Summary: Clearly articulate the problem, the affected code, and how your changes solve the problem.
  2. Complete the Impact Assessment: Provide YES/NO answers for all impact categories. Provide detailed explanations where necessary.
  3. Provide Detailed Testing Information: Specify your build host, target platform, and include actual testing logs demonstrating the issue and the fix.

@xiaoxiang781216 xiaoxiang781216 merged commit 01676cb into apache:master Oct 12, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Networking Effects networking subsystem Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants