acassis commented on PR #18340: URL: https://github.com/apache/nuttx/pull/18340#issuecomment-3843086238
> > > @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. > > @linguini1 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 #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 alt="image" width="528" height="697" src="https://private-user-images.githubusercontent.com/18066964/544464513-7d77b66d-ea79-4f68-9ee6-40380700ace7.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0NjQ1MTMtN2Q3N2I2NmQtZWE3OS00ZjY4LTllZTYtNDAzODA3MDBhY2U3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTMyOGQyNDJmYWU4M2RjYjZjMWMxZjFmYTdlYzA5NGQwMmU2ZmZiZjQ1MGRiNjE2NTI0NmQzNTJjZGZhODQwN2UmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.9 sf1Aj8VvwNHQdsgKet4LadjhSx_9Ccs0TnNitQolUU"> 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(#18224 and #18131), I made many comments before approving: <img alt="image" width="1041" height="186" src="https://private-user-images.githubusercontent.com/18066964/544429330-75109f30-146e-40f4-8446-a3e52235f010.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0MjkzMzAtNzUxMDlmMzAtMTQ2ZS00MGY0LTg0NDYtYTNlNTIyMzVmMDEwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTk2YWMzMDA0MjM1NDgxNDhlZDUwZjI1Y2FhM2IxMDQ3Nzc0MTYzMGM3M2ZjMzg2ZDQyN2Y3N2E2MjlkODdmZmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.rWcka07xeQYbBh8a-EqeK0FZmoe-ysGkqxGdDHxn7_I";> > > @wangchdo make the significant change to signal and hrtimer recently(#17642 and #17642), most comment come from me: <img alt="image" width="1061" height="187" src="https://private-user-images.githubusercontent.com/18066964/544435258-3eaaf420-27f2-4dac-94cf-220eb9cc79e5.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0MzUyNTgtM2VhYWY0MjAtMjdmMi00ZGFjLTk0Y2YtMjIwZWI5Y2M3OWU1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWRlN2NjYTY4NTAyNzQwOGVjNzk1NDBhMzI5NDhiZWFlMzc0ZjUwNGQwMGEyNjdiYmExMTcxOWZlYzI3NjhlNmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0._gARWLswmTHZPTIGM4GodVsxU6egUN7QqdKvigypho8";> > > Please do the constructive review, like @acassis @jerpelea (e.g. improve the pr organization, fix the commit message, or add the documentation). > > BTW, since the recent contribution rule change, Xiaomi team already decides ALL new features and components which doesn't tightly couple with NuttX will never be upstreamed in the future. > > If you still do review like this, my team will leave NuttX community and never upstream anymore. @acassis. @xiaoxiang781216 thanks for explained the details how your internal test works, but Matteo main point is the usage of AI is creating exaggerated text that doesn't help much during the CI. Unfortunately communication is not easy, even with two person that know each other and speak the same language, image in our case where we have "strange" persons from different places of the world. If @linguini1 asks for more testing, it is in good will, because although your team is testing it more than 10k times internally, your testing environment is different from NuttX mainline. It is so true because sometimes the code that passed on your rigorous test no even can compile when into mainline (it happened few days ago). So, until we get the automatic hardware testing working with the mainline, it is important to have a proof that the PR was tested using the mainline. And I know sometimes it could sound like a burden to your team, but this is the only way (before having the automated hardware testing in place) to guarantee that a PR is not breaking the mainline. -- 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]
![https://private-user-images.githubusercontent.com/18066964/544429330-75109f30-146e-40f4-8446-a3e52235f010.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0MjkzMzAtNzUxMDlmMzAtMTQ2ZS00MGY0LTg0NDYtYTNlNTIyMzVmMDEwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTk2YWMzMDA0MjM1NDgxNDhlZDUwZjI1Y2FhM2IxMDQ3Nzc0MTYzMGM3M2ZjMzg2ZDQyN2Y3N2E2MjlkODdmZmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.rWcka07xeQYbBh8a-EqeK0FZmoe-ysGkqxGdDHxn7_I"](https://private-user-images.githubusercontent.com/18066964/544429330-75109f30-146e-40f4-8446-a3e52235f010.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0MjkzMzAtNzUxMDlmMzAtMTQ2ZS00MGY0LTg0NDYtYTNlNTIyMzVmMDEwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTk2YWMzMDA0MjM1NDgxNDhlZDUwZjI1Y2FhM2IxMDQ3Nzc0MTYzMGM3M2ZjMzg2ZDQyN2Y3N2E2MjlkODdmZmQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.rWcka07xeQYbBh8a-EqeK0FZmoe-ysGkqxGdDHxn7_I"){kind=link}
![https://private-user-images.githubusercontent.com/18066964/544435258-3eaaf420-27f2-4dac-94cf-220eb9cc79e5.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0MzUyNTgtM2VhYWY0MjAtMjdmMi00ZGFjLTk0Y2YtMjIwZWI5Y2M3OWU1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWRlN2NjYTY4NTAyNzQwOGVjNzk1NDBhMzI5NDhiZWFlMzc0ZjUwNGQwMGEyNjdiYmExMTcxOWZlYzI3NjhlNmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0._gARWLswmTHZPTIGM4GodVsxU6egUN7QqdKvigypho8"](https://private-user-images.githubusercontent.com/18066964/544435258-3eaaf420-27f2-4dac-94cf-220eb9cc79e5.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NzAxNDQ1MDEsIm5iZiI6MTc3MDE0NDIwMSwicGF0aCI6Ii8xODA2Njk2NC81NDQ0MzUyNTgtM2VhYWY0MjAtMjdmMi00ZGFjLTk0Y2YtMjIwZWI5Y2M3OWU1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAyMDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjYwMjAzVDE4NDMyMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWRlN2NjYTY4NTAyNzQwOGVjNzk1NDBhMzI5NDhiZWFlMzc0ZjUwNGQwMGEyNjdiYmExMTcxOWZlYzI3NjhlNmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0._gARWLswmTHZPTIGM4GodVsxU6egUN7QqdKvigypho8"){kind=link}