On Tue, 17 Oct 2023 at 03:41, Sandro Santilli <s...@kbt.io> wrote: > > On Mon, Oct 16, 2023 at 09:59:35AM +1300, Nyall Dawson via QGIS-Developer > wrote: > > > If you flip the situation, you'll see that yes, you do have trust! > > > > - a complete stranger CANNOT approve their own changes > > - a complete stranger CANNOT approve other stranger's changes > > - a complete stranger CANNOT approve an approved member's changes > > > > vs > > > > - an approved member CANNOT approve their own changes > > - an approved member CAN approve a complete stranger's changes > > - an approved member CAN approve a another approved member's changes > > That's partial trust. I'm trusted to be able to judge someone else's > work but not to judge my own work !
Ok, it's partial trust. What's wrong with that? I wouldn't trust myself not to make accidental mistakes, or do something inefficient, or that I'm not duplicating code which is present elsewhere in QGIS. A second set of eyes over a pull request definitely reduces that risk. QGIS is MASSIVE, and none of us can keep a complete picture of the impact of code changes in our heads. Some real world examples, picked at semi-random (because they are recent): https://github.com/qgis/QGIS/pull/54941 : Submitted by a core developer. I reviewed, and it looked good to me. But then a third (!) set of eyes over the code has revealed a massive potential performance regression caused by the change (thanks Even!). https://github.com/qgis/QGIS/pull/54670: Submitted by a very experienced core developer. Code looks good at first pass, no visible bugs. But it's introducing a partial duplication of another utility function implemented elsewhere and exhaustively tested. The review identified that and as a result we avoided introducing an unnecessary, non trivial duplication of code. That's two examples. I'd very conservatively estimate that 1 in 10 approved PRs submitted by core committers have at least one set of changes suggested and implemented. Let's (pessimistically!) write half of those off as non-bugs, such as optimisation suggestions or changes relating to documentation/comments/code style. That's still **AT BEST** 1 in 20 pull requests submitted by core committers which needed fixes. In short: I'm more than OK with only partial trusting everyone. We ALL make mistakes, so let's stick with the processes we have which reduce the risk of these mistakes hitting end users.. Nyall > > Beside the same-company reviews things, consider I could always ask a > friend to file PRs for the sole purpose of me being able to merge them > in (I cannot merge my own...). The policy is flawed to me. > > --strk; _______________________________________________ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer