xiaoxiang781216 commented on PR #18340: URL: https://github.com/apache/nuttx/pull/18340#issuecomment-3842750439
> > @linguini1 I think @txy-21 and others add this just because you never comment the code change in all your review, but frequently ask people explain why he/she made this change. The description help you understand the code change and avoid ask the simple question. > > Okay. It doesn't help. I don't care what files are modified, and it doesn't help the review. The summary section is much more valuable, and this PR has a great summary! I don't understand the hostility or why you think I don't read any code in my reviews. Could you point out some pr, you have added comment to the real code change like me in the follow examples? > The first thing I look at is the PR description to get an idea of what the change is doing. > If there are no test logs, I don't bother reading the code. It could be untested for all I know, and that is a waste of review time. Let's summary how Xiaomi do the test internally before we upstream patch to community: 1. Each patch must pass CT(1000+ test cases) before it can be merged 2. DT(10000+ test cases) run every day to find the error which doesn't cache by CT 3. Run monkey test on 100 units sim/qemu/device more than 24 hours without crasah/hang 4. 10000+ manual test which run by tester manually before we make an official release every two/three months No test log doesn't mean the patch doesn't test or verifier. Most patch Xiaomi upstream is made in the last year and already pass the aboved test. Many bugs only happen in the special timing or scenario like https://github.com/apache/nuttx/pull/18266? when the error happen there is no log at all, how can @Otpvondoiatsdo provide you log? Do you print log after each statement? It's stupid to ask the log for each patch without look at the change! > If we're playing the blame game, I don't think you ever read the PR descriptions in any of your reviews, seeing as you constantly approve AI-generated PRs with no comments. All patch from Xiaomi team already reviewed by me many round before they can be merged into our internal repo, that's why I just approve them directly. Here is is an internal patch come from @szafonimateusz-mi <img width="528" height="697" alt="image" src="https://github.com/user-attachments/assets/7d77b66d-ea79-4f68-9ee6-40380700ace7"; /> 800+ line code reviewed by 30+ round and 100+ comments from Jan. 8 and not finally approved yet. @Fix-Point is one of my team member, he developed hrtimer patch on github directly(https://github.com/apache/nuttx/pull/18224 and https://github.com/apache/nuttx/pull/18131), I made many comments before approving: <img width="1041" height="186" alt="image" src="https://github.com/user-attachments/assets/75109f30-146e-40f4-8446-a3e52235f010"; /> @wangchdo make the significant change to signal and hrtimer recently(https://github.com/apache/nuttx/pull/17642 and https://github.com/apache/nuttx/pull/17642), most comment come from me: <img width="1061" height="187" alt="image" src="https://github.com/user-attachments/assets/3eaaf420-27f2-4dac-94cf-220eb9cc79e5"; /> Please do the constructive review, like @acassis @jerpelea (e.g. improve the pr organization, fix the commit message, or add the documentation). If you still do review like this, my team will leave NuttX community and never upstream any change in the future. @acassis. -- 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]
