Am Donnerstag, 28. März 2019, 09:29:22 CET schrieb Kevin Ottens: > Hello, > > On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote: > > Please note that the commits in this instance were pushed without > > review, so restrictions on merge requests wouldn't make a difference > > in this case unfortunately. > > Maybe it's about time to make reviews mandatory... I know it's unpopular in > KDE, and I advocated for "don't force a tool if you can get someone to look > at your screen or pair with you" in the past. Clearly this compromise gets > somewhat exploited and that's especially bad in the case of a fragile and > central component like KDE PIM.
Mandatory reviews in my experience only result in more fake reviews due to people pressuring each other to quickly get their simple patches reviewed, lowering the general quality of reviews. Also does the overhead reduce the number of minor improvements, where one (as qualified person) simply would have pushed in a minute a fix and get back to concentrate on the real work, instead of starting an overhead of having to juggle with yet another patch-under-review where the current work depends on. IMHO the actual problem here is: people do not do their post-push work and care for the state on CI. From what I saw, many breakages happened with reviewed patches. Whole releases even get done while CI is reporting failed builds, or at least lots of failing tests. I have no idea how to fix that. We would need to ask the people for whom this happens why it does happen, and how we can improve things so that CI checks become part of their workflow. Cheers Friedrich