From kde-core-devel Wed Apr 13 12:37:39 2022 From: Helio Chissini de Castro Date: Wed, 13 Apr 2022 12:37:39 +0000 To: kde-core-devel Subject: Re: Reviewing the process for giving people commit rights Message-Id: X-MARC-Message: https://marc.info/?l=kde-core-devel&m=164985350409840 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--0000000000003351a705dc8874ac" --0000000000003351a705dc8874ac Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable +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 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 > --0000000000003351a705dc8874ac Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
+1=C2=A0

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 giv= en
> > commit rights very soon after he appeared, and then immediately a= fter
> > that, he inappropriately merged a not-fully-reviewed an un-accept= ed
> > 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 committ= ing to
> > KDE repos, and giving him commit access was probably premature. >
> Well, the question this calls for is why the merge request was still n= ot
> fully reviewed almost six weeks after submission. I would guess that t= hat is
> what the misunderstanding came from: the submitter most likely assumed= that
> the changes were fine given that there were no outstanding comments. (= The
> submitter did try to address those comments that you had in those six<= br> > weeks.)
>
> I should also point out that the complaints in Xaver Hugl's post-m= erge
> review were all only formatting/whitespace, choice of comment sign, an= d
> 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 t= he
hooks also complain about formatting issues in unstaged/uncommitted code, e= .g.
temporarily commented out code where the commented out code is not correctl= y
indented).

Formatting is something no reviewer should have to waste brain energy on nowadays.

Regards,
Ingo
--0000000000003351a705dc8874ac--