Re: [PR] boards/defconfig: remove unused config about dd [nuttx]
xiaoxiang781216 commented on PR #16198: URL: https://github.com/apache/nuttx/pull/16198#issuecomment-2801826795 > Hmm this one looks only like removing `dd` from defconfigs? So the real `dd` replacement is somewhere else? since system dd is enable automatically in no small config: https://github.com/apache/nuttx-apps/pull/3057/files#diff-9745c2aa3cbde9afc05370bbba5eb67b7bc7b98d1330684ae546de7fb762861fR8 > I wonder if it is really necessary to remove built-in nsh dd and not just add external dd as another option (both should be exlusive that way)? then, both implementation will unsync, which already happen now, please review https://github.com/apache/nuttx-apps/pull/3057 to find the difference. -- 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]
Re: [PR] boards/defconfig: remove unused config about dd [nuttx]
cederom commented on PR #16198: URL: https://github.com/apache/nuttx/pull/16198#issuecomment-2801810503 Hmm this one looks only like removing `dd` from defconfigs? So the real `dd` replacement is somewhere else? I wonder if it is really necessary to remove built-in nsh dd and not just add external dd as another option (both should be exlusive that way)? -- 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]
Re: [PR] boards/defconfig: remove unused config about dd [nuttx]
lupyuen commented on PR #16198: URL: https://github.com/apache/nuttx/pull/16198#issuecomment-2801677631 Hi @Donny9: Because the Normal CI Checks won't work for this Breaking PR, I'm now running the Special CI Checks (see below) on our NuttX Build Farm. The CI Checks will complete in 36 hours, the CI Logs will appear here: https://gist.github.com/nuttxpr https://github.com/lupyuen/nuttx-release/blob/main/run-ci-special.sh ```bash ## Repeat forever for All CI Jobs for (( ; ; )); do for job in \ arm-01 arm-02 arm-03 arm-04 \ arm-05 arm-06 arm-07 arm-08 \ arm-09 arm-10 arm-11 arm-12 \ arm-13 arm-14 \ arm64-01 \ other \ risc-v-01 risc-v-02 risc-v-03 risc-v-04 \ risc-v-05 risc-v-06 risc-v-07 \ sim-01 sim-02 sim-03 \ x86_64-01 \ xtensa-01 xtensa-02 xtensa-03 do ## Run the CI in Docker Container ## If CI Test Hangs: Kill it after 3 hours sudo docker run -it \ ghcr.io/apache/nuttx/apache-nuttx-ci-linux:latest \ /bin/bash -c " set -x ; uname -a ; cd ; pwd ; git clone https://github.com/Donny9/incubator-nuttx nuttx --branch sem ; git clone https://github.com/Donny9/incubator-nuttx-apps apps --branch system_dd ; pushd nuttx ; git reset --hard HEAD ; echo NuttX Source: https://github.com/apache/nuttx/tree/\$(git rev-parse HEAD) ; popd ; pushd apps ; git reset --hard HEAD ; echo NuttX Apps: https://github.com/apache/nuttx-apps/tree/\$(git rev-parse HEAD) ; popd ; sleep 10 ; cd nuttx/tools/ci ; ( sleep 10800 ; echo Killing pytest after timeout... ; pkill -f pytest )& (./cibuild.sh -c -A -N -R testlist/$job.dat || echo '* BUILD FAILED') ; " done ``` -- 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]
Re: [PR] boards/defconfig: remove unused config about dd [nuttx]
lupyuen commented on PR #16198: URL: https://github.com/apache/nuttx/pull/16198#issuecomment-2801225788 Hi @Donny9: Since this is a Breaking Change, could you please follow the Breaking Change Handling Process, and start the voting on the Mailing List? Thanks :-) https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md#113-breaking-changes-handling-process (1) Please remember to indicate that voting will be open for 72 hours (2) Vote +1 to Accept the Breaking PR, -1 to Reject the Breaking PR (So that we won't have -1 votes concerning other issues) -- 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]
Re: [PR] boards/defconfig: remove unused config about dd [nuttx]
lupyuen commented on PR #16198: URL: https://github.com/apache/nuttx/pull/16198#issuecomment-2800202495 Sorry @cederom : Should we consider this PR a Breaking Change? Because CI Jobs for NuttX Kernel won't succeed, unless we patch NuttX Apps? 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] boards/defconfig: remove unused config about dd [nuttx]
nuttxpr commented on PR #16198: URL: https://github.com/apache/nuttx/pull/16198#issuecomment-271334 [**\[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 summary and points to a related PR for impact, it lacks crucial details. Specifically: * **Summary:** Needs more detail. *Why* is the dd config being removed? Is it obsolete, redundant, causing a conflict, etc.? How exactly is it being removed (deleted, commented out, etc.)? * **Impact:** Linking to another PR is insufficient. The impact needs to be explicitly stated within the PR itself. While the linked PR might provide context, reviewers shouldn't have to chase down external links to understand the impact. Explicitly address all the impact checklist items (user, build, hardware, documentation, security, compatibility). * **Testing:** "sim:nsh" is inadequate. What tests were run? What were the *results* of the tests, both before and after the change? Provide concrete log output or descriptions of observed behavior demonstrating that the change works as intended and hasn't introduced regressions. Just stating the target platform doesn't prove anything. In short, the PR needs to be significantly more detailed to meet the NuttX requirements. -- 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]
