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

fs/poll: add debug log for poll/epoll failed #13697

Merged
merged 2 commits into from
Sep 28, 2024
Merged

Conversation

Donny9
Copy link
Contributor

@Donny9 Donny9 commented Sep 27, 2024

Summary

fs/poll: add debug log for poll/epoll failed

Impact

N/A

Testing

Vela

@github-actions github-actions bot added Area: File System File System issues Size: S The size of the change in this PR is small labels Sep 27, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 27, 2024

[Experimental Bot, please feedback here]

This PR appears incomplete based on the NuttX requirements.

Here's why and what's missing:

  • Summary:
    • Missing:
      • A clear explanation of why the debug logs are necessary. What problem are they solving?
      • Details on how the debug logs work - what information is logged, and under what conditions?
  • Impact:
    • Potentially inaccurate: Even debug logs can have an impact. Consider:
      • Performance: Will the logs impact performance, especially under heavy load?
      • Storage: Where are the logs stored? Could they fill up limited storage?
      • Build: While the code change might be small, will it require any build system tweaks to enable/disable debug logging?
  • Testing:
    • Insufficient: "Vela" alone is not descriptive enough.
      • Which Vela build targets were used? Be specific about the architectures and boards.
      • Provide snippets of the actual log output before and after the change to demonstrate the added debug information.

In short, the PR needs more context and detail to be properly evaluated.

@xiaoxiang781216 xiaoxiang781216 merged commit 29e7b00 into apache:master Sep 28, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: File System File System issues 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