Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
lupyuen commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2691734844 @keever50 We're seeking at least 2 reviewers, according to the latest guidelines below. Let's wait a day or two for more reviewers before we merge this. Thanks :-) https://lists.apache.org/thread/v5b8gow2pfbzoformwn0dm84ypy7oysn -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
cederom commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2691863838 Thanks @keever50 :-) * Builds for me on FreeBSD but have no SPI to test :-) * Would be nice to present runtime logs of working solution :-) -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
acassis merged PR #15894: URL: https://github.com/apache/nuttx/pull/15894 -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2691420358 @lupyuen Thank you for sharing that! However i am curious. How do PR reviews usually go? Do we wait till enough reviewers spotted the PR by luck? Or do i need to ping reviewers? This one is hanging around for a little bit now, so i hope i'm not waiting while i am supposed to do something :) -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2690195968 @lupyuen looks like it worked now but with 3 skips i see -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
lupyuen commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2683394620 There's a download error, I restarted the CI Checks. ``` End-of-central-directory signature not found. Either this file is not a zipfile, or it constitutes one disk of a multi-part archive. In the latter case the central directory and zipfile comment will be found on the last disk(s) of this archive. ``` -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2682943620 I'm not sure how i fix this one... It does not seem related to what i just pushed? `/github/workspace/sources/nuttx/tools/testbuild.sh: line 385: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory` -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
lupyuen commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677910306 Looks great, thanks! :-) -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677909757 oh okay do the same thing twice and it works. -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677906247 ahhh i see signed off now! However now i have issues pushing it because it does not let me enter my password anymore. Ill get back to this soon -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677900199 @lupyuen By just running commit -s i get "nothing to commit, working tree clean". I ran git commit --amend --no-edit -S which should in theory add a sign? -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
lupyuen commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677899773 Hmmm I don't see the "Signed-off-by". Could you do `git push`? Thanks! -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677883271 Signed -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
lupyuen commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677878629 Sorry could you run `git commit -s` to sign off? Thanks :-) -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677874634 I realized i copied the entire PR message and should have been only summery i believe. Hope this isnt a big issue. -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
keever50 commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677870987 Okay! Signed and added a message -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
lupyuen commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2677236562 Please fill in the Commit Description (copy from the PR Summary). And remember to sign-off with `git commit -s`. Thanks! :-) - https://github.com/apache/nuttx/pull/15894/commits/93e0db1461a2956cc4930c9c452deb414fe8a37d -- 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
Re: [PR] boards/arm/rp2040/common: Add weak_function to SPI common logic [nuttx]
nuttxpr commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2676441776 [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR does not fully meet the NuttX requirements. While it provides a good starting point, it lacks crucial details. **Missing/Insufficient Information:** * **Summary:** While it mentions the "what," it's missing a strong "why." Why is this change *necessary*? What specific problem does it solve beyond a general desire for extensibility? A related NuttX issue would greatly strengthen this section. * **Impact:** The impact section is too vague. * **User Impact:** While it mentions custom boards, it doesn't specify *how* users would adapt. Would they need to change configurations? Write new drivers? Clearer examples are needed. * **Build Impact:** Does adding `weak_function` change the build process in any way? Even if the answer is "NO," explicitly state it. * **Hardware Impact:** Specify the affected architecture (ARM) and board (Raspberry Pi Pico, and any others). "Custom board" is not specific enough. List the *specific* hardware changes if any. * **Documentation Impact:** Does this require documentation updates? If so, have they been included in the PR? * **Security Impact:** Explicitly state "NO" if there's no security impact. * **Compatibility Impact:** Explicitly state "NO" or explain potential backward compatibility issues. Consider the impact on existing boards using the RP2040 SPI. * **Testing:** * **Build Host(s):** Be specific about the Linux distribution used, the compiler version (e.g., GCC 12.2.0), and any relevant build tools. * **Target(s):** Be precise about the target configurations tested. "raspberrypi-pico:nsh config" isn't precise enough. Specify the full configuration name (e.g., `configs/raspberrypi-pico/nsh`). For the custom board, provide details about its configuration. * **Logs:** The "Testing logs" sections are empty. Include relevant log output demonstrating the functionality before and after the change. Show how the new weak functions are being overridden and what the results are. Demonstrate that existing functionality is not broken. **Recommendations:** 1. **Clarify the "why":** Explain the specific problem this solves. Provide a concrete example of how the lack of this feature hindered development before. Link a related NuttX issue if one exists. 2. **Detailed Impact Assessment:** Be explicit about all impact categories, even if the answer is "NO." Provide specific details and examples where necessary. 3. **Thorough Testing Information:** Provide complete details about the build host and target environments. Include relevant log output demonstrating the change's functionality and the absence of regressions. Test and document the behavior when the weak functions are *not* overridden to ensure backward compatibility. 4. **Compiler Compatibility:** Research and address the compiler compatibility concerns regarding `weak_function`. Document any limitations or required compiler versions. By addressing these points, the PR will better meet the NuttX requirements and be more likely to be accepted. -- 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
