On Thu, Mar 28, 2019, 6:36 AM Friedrich W. H. Kossebau <kosse...@kde.org> wrote:
> 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. > Then fix what's broken. If these projects need manditory reviews fine but don't take a one-size-fits-all approach. > > > 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. > Agreed. > > 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. > Requiring pre-commit hooks which run these could be helpful. They could stop this at the local machine. Perhaps also a reminder to check ci. Not sure this completely solves the issue but it would be workable for small projects like kdiff3 and would reduce overhead for minor typo correction. > > 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 > > >