<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 28, 2019, 6:36 AM Friedrich W. H. Kossebau <<a href="mailto:kossebau@kde.org">kossebau@kde.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Am Donnerstag, 28. März 2019, 09:29:22 CET schrieb Kevin Ottens:<br>
> Hello,<br>
> <br>
> On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:<br>
> > Please note that the commits in this instance were pushed without<br>
> > review, so restrictions on merge requests wouldn't make a difference<br>
> > in this case unfortunately.<br>
> <br>
> Maybe it's about time to make reviews mandatory... I know it's unpopular in<br>
> KDE, and I advocated for "don't force a tool if you can get someone to look<br>
> at your screen or pair with you" in the past. Clearly this compromise gets<br>
> somewhat exploited and that's especially bad in the case of a fragile and<br>
> central component like KDE PIM.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Then fix what's broken. If these projects need manditory reviews fine but don't take a one-size-fits-all approach. </div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><br>
Mandatory reviews in my experience only result in more fake reviews due to <br>
people pressuring each other to quickly get their simple patches reviewed, <br>
lowering the general quality of reviews.<br>
Also does the overhead reduce the number of minor improvements, where one (as <br>
qualified person) simply would have pushed in a minute a fix and get back to <br>
concentrate on the real work, instead of starting an overhead of having to <br>
juggle with yet another patch-under-review where the current work depends on.<br>
<br>
IMHO the actual problem here is: people do not do their post-push work and <br>
care for the state on CI.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Agreed.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
>From what I saw, many breakages happened with reviewed patches. Whole <br>
releases even get done while CI is reporting failed builds, or at least lots <br>
of failing tests.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I have no idea how to fix that. We would need to ask the people for whom this <br>
happens why it does happen, and how we can improve things so that CI checks <br>
become part of their workflow.<br>
<br>
Cheers<br>
Friedrich<br>
<br>
<br>
</blockquote></div></div></div>