nuttxpr commented on PR #15131: URL: https://github.com/apache/nuttx/pull/15131#issuecomment-2534260947
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the basic NuttX requirements, although it's minimally documented. It provides context by referencing a previous commit, explaining the overall goal (removing `spin_lock_irqsave(NULL)`), and indicating where the changes are. However, more detail would be beneficial. Here's how it could be improved: **Summary:** * Be more explicit about *what* this PR changes within `cxd56_rtc.c`. Does it introduce a new lock? Replace an existing lock? What code paths are affected? * Briefly explain *how* the new lock mechanism works (if applicable). Is it a mutex? Semaphore? What are the protected resources? * The commit message mentions a reason, but the PR should explicitly state the problem `spin_lock_irqsave(NULL)` was causing. **Impact:** * "RELEASE" is not sufficient. Address each point specifically, even if the answer is "NO". If there's a potential for user impact, backward compatibility issues, etc., explain why "RELEASE" is the correct assessment. This justifies the impact assessment and prevents misunderstandings. **Testing:** * "CI" is insufficient. While CI is important, local testing is still required. Specify the host and target environments where testing was performed. * Provide *specific* test cases and their expected outcomes before and after the change, not just "CI." Ideally, there would be some evidence in the logs showing the lock in action and demonstrating the absence of the previous issue. If logs are lengthy, summarize the relevant portions. Even though it references a prior commit, this PR should be self-contained and understandable without requiring the reviewer to hunt down other information. Greater detail and explicit testing results would strengthen the PR significantly. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
