yorik wrote: ↑
Mon Jun 15, 2020 9:52 am
I am myself not skilled enough with C++ to decide on Realthunder's code, and I believe nobody is (except Werner maybe). But any of us can help with testing. Werner doesn't do differently, AFAICS. If we test a PR and can write a review saying that we tested and report what we found, we can help greatly the process to go forward. If several of us (specially C++ people please) do test and approve the code, I think we could build enough trust to merge with far less need of Werner's work.
I do not think it is merely a matter of c++ skill. I can understand his code. I think many of us can.
It is not a problem either of him making programing mistakes (leaking memory, lack of index checks,...). He does know programming.
What makes RT's requests hard to review is IMO that most of it touches not only the modules (/Mod), but the core. They are also sometimes extensive in scope.
To decide whether these changes are acceptable or not, one has to have a very very deep knowledge of FC's architecture, not only about how it works, but about the design decisions that were made in the past. There are things that look just fine and then there are, for example, performance hits. I merged one of his PR-ed a month ago, and performance issues were only discovered because of Werner's attentive eye to what the rest of us do (including this integrator in practice).
My knowledge of FC's architecture has improve substantially over the years. I have also improved my knowledge about software architecture and patterns. Yet, I have not have had the opportunity to put in practice much of that theoretical knowledge and I have still a lot of room for improvement.
This said. I can volunteered to review RT's PR's, if Werner is ok with it.
1) I can report what I find and why I think the code is acceptable or not, or where I think problems may arise. Then let to Werner the final merge action.
2) If Werner asks me to, I can also directly merge what I think it is acceptable with a high confidence, while leaving to him what does not have a high confidence.
This is up to some extent similar to the standard I apply to myself. When I have a high confidence I merge my own PRs. When I think my changes might have a higher impact than what I envision, I ask Werner to review my code before merge.
If I merge RT's PRs to accelerate integration work (of course after a thorough review), then we still need to count with the (higher than we are used with Werner) probability of having to revert things. At the end of the day, I can offer what I have. I can try to imitate Werner, but I cannot pretend to be at his level.
Let me know.