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

Add support fchstat and chstat function for littlefs #13964

Merged
merged 7 commits into from
Oct 11, 2024

Conversation

guohao15
Copy link
Contributor

@guohao15 guohao15 commented Oct 9, 2024

Summary

add functions for littlefs :

static int littlefs_fchstat(FAR const struct file *filep,
FAR const struct stat *buf, int flags);
static int littlefs_chstat(FAR struct inode *mountpt,
FAR const char *relpath,
FAR const struct stat *buf, int flags);

Impact

littlefs_vfs

Testing

NuttxSyscallTestSuites
NuttxFsTestSuites

@github-actions github-actions bot added Area: File System File System issues Size: M The size of the change in this PR is medium labels Oct 9, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 9, 2024

[Experimental Bot, please feedback here]

The provided PR description is missing some key information to determine if it meets NuttX requirements.

Here's a breakdown:

Summary:

  • Insufficient: While it mentions adding functions to littlefs, it doesn't explain why this change is necessary.
    • Is it a fix for a bug?
    • Is it a new feature to improve functionality or performance?
    • What problem does it solve?
  • Unclear: The description of "how the change works" is vague.
    • What do these functions specifically do?
    • How do they interact with the rest of the littlefs code?
  • Missing: There's no mention of related issues. Are there any open issues this PR addresses?

Impact:

  • Too Vague: Simply stating "littlefs_vfs" doesn't explain the impact.
    • Be specific about which aspects of littlefs_vfs are affected.
  • Incomplete: The PR needs to address all impact points listed in the requirements. Provide "NO" or a detailed "YES" explanation for each:
    • Impact on user
    • Impact on build
    • Impact on hardware
    • Impact on documentation
    • Impact on security
    • Impact on compatibility

Testing:

  • Insufficient: "NuttxSyscallTestSuites" and "NuttxFsTestSuites" are too broad.
    • List the specific test cases you ran to verify the changes.
  • Missing Logs: You must provide actual testing logs (before and after the change) to demonstrate the issue and the fix.

To improve the PR:

  1. Expand the Summary: Clearly articulate the "why," "what," and "how" of your changes.
  2. Thoroughly Address Impact: Provide specific answers (with explanations) for each impact category.
  3. Provide Detailed Testing Information:
    • Specify the exact test cases executed.
    • Include the actual "before" and "after" testing logs.

By providing this missing information, you'll make it much easier for reviewers to assess your PR and determine if it meets NuttX's standards.

@guohao15 guohao15 force-pushed the pick-lfs branch 4 times, most recently from 574686c to e0fb1bb Compare October 10, 2024 12:31
DS-LK and others added 4 commits October 11, 2024 10:18
Signed-off-by: zhouliang3 <zhouliang3@xiaomi.com>
Signed-off-by: zhouliang3 <zhouliang3@xiaomi.com>
Signed-off-by: zhouliang3 <zhouliang3@xiaomi.com>
Signed-off-by: Jiuzhu Dong <dongjiuzhu1@xiaomi.com>
DS-LK and others added 3 commits October 11, 2024 10:49
Signed-off-by: zhouliang3 <zhouliang3@xiaomi.com>
Signed-off-by: ligd <liguiding1@xiaomi.com>
mkdir /data/log   success
mkdir /data/log/  failed  in littlefs (but fatfs/yaffs/tmpfs success)

Signed-off-by: guohao15 <guohao15@xiaomi.com>
@GUIDINGLI GUIDINGLI merged commit 69f3774 into apache:master Oct 11, 2024
37 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: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants