[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Reviewing the process for giving people commit rights
From:       Helio Chissini de Castro <helio () kde ! org>
Date:       2022-04-13 12:37:39
Message-ID: CAKPiqoE442OpH1=qLBWYmDyohR33UQUpnC59QMZx_Uxw1-C4Qw () mail ! gmail ! com
[Download RAW message or body]

+1

S=C3=B3 far, simple solution and would reduce a lot work of reviewers.

On Wed, Apr 13, 2022, 04:27 Ingo Kl=C3=B6cker <kloecker@kde.org> wrote:

> On Samstag, 2. April 2022 11:21:11 CEST Kevin Kofler wrote:
> > Nate Graham wrote:
> > > This caused a problem recently in KWin. A new contributor was given
> > > commit rights very soon after he appeared, and then immediately after
> > > that, he inappropriately merged a not-fully-reviewed an un-accepted
> > > merge request
> > > (https://invent.kde.org/plasma/kwin/-/merge_requests/1980). It seems
> > > that he did not have a sense of the cultural norms around committing =
to
> > > KDE repos, and giving him commit access was probably premature.
> >
> > Well, the question this calls for is why the merge request was still no=
t
> > fully reviewed almost six weeks after submission. I would guess that
> that is
> > what the misunderstanding came from: the submitter most likely assumed
> that
> > the changes were fine given that there were no outstanding comments. (T=
he
> > submitter did try to address those comments that you had in those six
> > weeks.)
> >
> > I should also point out that the complaints in Xaver Hugl's post-merge
> > review were all only formatting/whitespace, choice of comment sign, and
> > brace issues (with no effect on the end user at all),
>
> Several PIM libraries have clang-format pre-commit hooks that prevent
> formatting issues in the first place (and, occasionally, annoy me because
> the
> hooks also complain about formatting issues in unstaged/uncommitted code,
> e.g.
> temporarily commented out code where the commented out code is not
> correctly
> indented).
>
> Formatting is something no reviewer should have to waste brain energy on
> nowadays.
>
> Regards,
> Ingo
>

[Attachment #3 (text/html)]

<div dir="auto">+1  <div dir="auto"><br></div><div dir="auto">Só far, simple \
solution and would reduce a lot work of reviewers.</div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 13, 2022, 04:27 \
Ingo Klöcker &lt;<a href="mailto:kloecker@kde.org">kloecker@kde.org</a>&gt; \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">On Samstag, 2. April 2022 11:21:11 \
CEST Kevin Kofler wrote:<br> &gt; Nate Graham wrote:<br>
&gt; &gt; This caused a problem recently in KWin. A new contributor was given<br>
&gt; &gt; commit rights very soon after he appeared, and then immediately after<br>
&gt; &gt; that, he inappropriately merged a not-fully-reviewed an un-accepted<br>
&gt; &gt; merge request<br>
&gt; &gt; (<a href="https://invent.kde.org/plasma/kwin/-/merge_requests/1980" \
rel="noreferrer noreferrer" \
target="_blank">https://invent.kde.org/plasma/kwin/-/merge_requests/1980</a>). It \
seems<br> &gt; &gt; that he did not have a sense of the cultural norms around \
committing to<br> &gt; &gt; KDE repos, and giving him commit access was probably \
premature.<br> &gt; <br>
&gt; Well, the question this calls for is why the merge request was still not<br>
&gt; fully reviewed almost six weeks after submission. I would guess that that is<br>
&gt; what the misunderstanding came from: the submitter most likely assumed that<br>
&gt; the changes were fine given that there were no outstanding comments. (The<br>
&gt; submitter did try to address those comments that you had in those six<br>
&gt; weeks.)<br>
&gt; <br>
&gt; I should also point out that the complaints in Xaver Hugl&#39;s post-merge<br>
&gt; review were all only formatting/whitespace, choice of comment sign, and<br>
&gt; brace issues (with no effect on the end user at all),<br>
<br>
Several PIM libraries have clang-format pre-commit hooks that prevent <br>
formatting issues in the first place (and, occasionally, annoy me because the <br>
hooks also complain about formatting issues in unstaged/uncommitted code, e.g. <br>
temporarily commented out code where the commented out code is not correctly <br>
indented).<br>
<br>
Formatting is something no reviewer should have to waste brain energy on <br>
nowadays.<br>
<br>
Regards,<br>
Ingo<br>
</blockquote></div>



[prev in list] [next in list] [prev in thread] [next in thread]