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/inode: refresh tcb after each file sync() is completed #14444

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anchao
Copy link
Contributor

@anchao anchao commented Oct 22, 2024

Summary

fs/inode: refresh tcb after each file sync() is completed

Impact

N/A

Testing

ci-check

@anchao anchao marked this pull request as draft October 22, 2024 05:58
@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 Oct 22, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 22, 2024

[Experimental Bot, please feedback here]

This PR appears incomplete for a NuttX pull request based on the provided requirements. Here's why:

  • Summary Lacks Details:
    • Why? The "why" of the change is not explained. What problem does refreshing the TCB after each sync() solve?
    • How? A high-level description of how the change is implemented is missing.
  • Impact Overly Simplified:
    • While marking everything as "N/A" might be technically correct, it's unlikely a code change has zero impact. Consider these carefully:
      • Compatibility: Does this change in sync() behavior potentially break existing applications that relied on the old behavior?
      • Performance: Does frequently refreshing the TCB introduce any performance overhead?
  • Testing Insufficient:
    • "ci-check" is not enough. You need to provide concrete testing logs or commands demonstrating the issue before the change and the improvement after the change.
    • Target Specificity: Which architectures and boards did you test on?

To improve this PR:

  1. Expand the Summary: Explain the rationale, the specific problem being solved, and briefly outline the implementation approach.
  2. Review Impact Carefully: Be thorough; even minor impacts should be mentioned. For "NO" answers, a brief justification is helpful (e.g., "Impact on build: NO - Changes are localized to the fs/inode directory").
  3. Provide Detailed Testing Information:
    • Include actual commands or steps to reproduce the issue before your change.
    • Show the output or logs demonstrating the problem.
    • Do the same for the fixed scenario, showcasing the improvement.
    • Be specific about the testing environment (host OS, compiler, target architecture, board configuration).

@acassis
Copy link
Contributor

acassis commented Oct 27, 2024

@anchao please include a commit log message with more info about this modification

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