Yes, for hardware related problems a hardware verification seems mandatory. Also we should define a minimal required test sequence (i.e. run nsh, uname -a, ostest). Maybe this can be done with some dedicated "selftest" configuration added to all boards?
This PR broke something: https://github.com/apache/nuttx/pull/15437 This PR fixed that thing: https://github.com/apache/nuttx/pull/15748 I am sure updates are done in good faith. I requested running ostest from the PR author in addition to nsh. I know the nsh runs. But how can we be sure other things are still not broken? In the meantime @anchao approved the PR and merged code. Not waiting for the ostest results. And so the initial situation may repeat, after some time, someone will angrily report something else is broken. I think Sebastien is right. We should have more respect to code repository, its not a scratchpad. This leads me to conclusion we should implement zero trust to user testing, there must be an objective runtime "selftest" procedure. Also I think we should increase required reviewers from 2 to 4 for better cross examination. This may also activate our community. Tomek On Mon, Feb 3, 2025 at 6:36 PM Matteo Golin <matteo.go...@gmail.com> wrote: > > I like this plan of requiring hardware testing. Also, even if the PR may > contain enough information to be justified, it becomes really difficult for > anyone debugging to track down changes if the PR body doesn't have a > description of any testing or the change impact. If all the PRs follow the > enforced format it becomes so much easier to debug issues because we can > expect all PRs to be in the same format and immediately know where to look. > > On Mon, Feb 3, 2025 at 11:57 AM Nathan Hartman <hartman.nat...@gmail.com> > wrote: > > > Maybe we shouldn't merge PRs until at least one reviewer tests on real > > hardware. It doesn't have to be the same board, but at least the same arch, > > if it touches arch code. If it's in sched or something shared on all > > architectures then it's okay to test on any arch. We would need to decide > > how to handle the cases where no reviewer has the same arch. > > > > On Mon, Feb 3, 2025 at 10:09 AM <michal.lyszc...@bofc.pl> wrote: > > > > > On 2025-02-03 22:45:29, Xiang Xiao wrote: > > > > The ai bot is a soft suggestion, not a rule from Lup introduces it > > > > initially. > > > > Many PR already contain the useful information, like these: > > > > https://github.com/apache/nuttx/pull/15749 > > > > https://github.com/apache/nuttx/pull/15728 > > > > But ai bot still complain the information isn't enough. > > > > Even https://github.com/apache/nuttx/pull/15437 already contains the > > > enough > > > > information to explain why this change is made. > > > Yep, especially this section of PR perfectly explains steps performed > > > to not break anything: > > > > > > > Testing > > > > > > > > Update this section with details on how did you verify the change, > > > > what Host was used for build (OS, CPU, compiler, ..), what Target was > > > > used for verification (arch, board:config, ..), etc. Providing build > > > > and runtime logs from before and after change is highly appreciated. > > > > > > That PR touched WHOLE cpu family, and yet not a single test was performed > > > on real hardware it seems. > > > > > > -- > > > > > > > > .-----------------.-------------------.----------------------.-----------------. > > > | Michal Lyszczek | Embedded C, Linux | Company Address | .-. > > > opensource | > > > | +48 727 564 419 | Software > > > < > > https://www.google.com/maps/search/7+564+419+%7C+Software?entry=gmail&source=g > > > > > > Engineer | Akacjowa 10a; 55-330 | oo| supporter | > > > | https://bofc.pl `----.--------------: Brzezinka Sredzka PL | /`'\ > > > & | > > > | GPG FF1EBFE7E3A974B1 | Bits of Code | NIP: 813 349 58 78 |(\_;/) > > > programer | > > > > > > > > `----------------------^--------------^----------------------^-----------------' > > > > > -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
