nuttxpr commented on PR #14844: URL: https://github.com/apache/nuttx/pull/14844#issuecomment-2484624373
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the NuttX requirements, although some minor improvements could be made. Here's a breakdown: **Strengths:** * **Clear Summary:** The summary explains the problem, the solution, and links to the relevant discussion. * **Impact Assessment:** The impact section adequately addresses most points, confirming no significant changes to build, hardware, documentation, or compatibility. * **Testing Information:** Provides basic testing information, including host and target. The output demonstrates the intended effect of the change. **Areas for Improvement:** * **More Detail in Summary:** While the summary is good, it could be slightly more explicit about *how* ignoring `-ENOENT` solves the problem. For example, mention that the `fstat` interface will now return default values instead of an error in the case of missing attributes. * **Explicit "Is existing feature changed?":** In the Impact section, explicitly state "YES" or "NO" for "Is existing feature changed?". Given the description, the answer is likely YES. Clarify which feature is changed (the `fstat` behavior). * **More Robust Testing:** The testing is minimal. Ideally, it should include: * Tests with and without `CONFIG_FS_LITTLEFS_ATTR_UPDATE` enabled. * Tests that create files, set attributes, and then verify the attributes via `fstat`. * Negative tests (e.g., trying to access attributes that haven't been set, with and without the new macro). * Consider adding a test case to the NuttX test suite. * **"Testing logs before change":** Include actual logs (even if they show the error). This helps demonstrate the improvement. **Conclusion:** While the PR is generally well-structured, adding more detail, especially in the testing section, will significantly strengthen it and make review easier. The minor improvements suggested above will make the PR more complete and robust. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
