linguini1 commented on PR #18500: URL: https://github.com/apache/nuttx/pull/18500#issuecomment-4010270806
Thanks for the feedback everyone! I guess the goal of this PR was: 1) get some feedback on the tool 2) avoid a CI change all at once But, I think the concerns are right that it is probably better to test this tool in the integrated CI before merging it; maybe that will catch corner cases earlier. I will use Lup's handy trick to test this on my own fork beforehand :) @lupyuen to answer your questions: 1) running it on old PRs as a sanity check is a good idea! Maybe if I get this working on my own fork, I can try it against the new patches coming into mainline and compare against the existing CI 2) Yes, probably! To my (limited) knowledge, the builds to be run are selected in the arch workflow and then output to the Linux build workflow. I think we could do some check like "if arch is arm64, instead of using arm64-01, use the new script to determine the builds". 3) Unfortunately in its current state, no :( it only adds benefit to PRs which aren't "complex" (as you term them in your video). It can reduce full arch builds to just board builds depending on the locality of the changes. Apps, drivers, common code, etc. are still complex PRs and require the full run. 4) I will have to monitor it if/when it is merged. Which is perhaps why it's a good idea to follow your suggestion of phasing it into one arch at a time; that will avoid major breakages. 5) This is true! But, this tool doesn't avoid running any builds that can't be avoided (at least, to the best of my knowledge) as changes that are exclusively local to a chip can be entirely tested by building defconfigs associated with that chip only. I.e., no point running BCM2711 builds when changes were made to the Pinephone only! So, it should still preserve the quality of testing. 6) I think you and @raiden00pl are right about this! I'll test it on my own fork using your suggested method and if things still go smoothly, we can revisit a PR! @cederom if we follow Lup's suggestion of phasing in this tool one arch at a time, then it should be easy to roll back if something goes terribly wrong. Hopefully by that point it will have been significantly tested. -- 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]
