All of lore.kernel.org
 help / color / mirror / Atom feed
From: Han-Wen Nienhuys <hanwen@google.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
	Dmitry Vyukov <dvyukov@google.com>,
	Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	Laura Abbott <labbott@redhat.com>,
	Don Zickus <dzickus@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Daniel Axtens <dja@axtens.net>,
	David Miller <davem@davemloft.net>, Drew DeVault <sir@cmpwn.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	workflows@vger.kernel.org
Subject: Re: thoughts on a Merge Request based development workflow
Date: Tue, 15 Oct 2019 20:58:14 +0200	[thread overview]
Message-ID: <CAFQ2z_N0Hriv68HajKZoNg1AyXzd_GqT8a2sDoCRZkUJy_vKSA@mail.gmail.com> (raw)
In-Reply-To: <20191015160712.GD1003139@kroah.com>

On Tue, Oct 15, 2019 at 6:07 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > Gerrit isn't a big favorite of many people, but some of that
> > perception may be outdated. Since 2016, Google has significantly
> > increased its investment in Gerrit. For example, we have rewritten the
> > web UI from scratch, and there have been many performance
> > improvements.
>
> As one of the many people who complain about Gerrit (I gave a whole talk
> about it!), I guess I should comment here...
>..
> Anyway, my objections:
>         - slow.  Seriously, have you tried using it on a slower network
>           connection (i.e. cellular teather, or train/bus wifi, or cafe
>           wifi?)

I did (train wifi), and found it to be workable, but it might be that
I have different expectations for speed.  If you're worried about
off-line behavior, you could run a local mirror of an Gerrit server,
and posting draft comments locally, and then use a script to post the
draft comments to the central gerrit server once you're online.

>         - Can not see all changes made in a single commit across
>           multiple files on the same page.  You have to click through
>           each and every individual file to see the diffs.  That's
>           horrid for patches that touch multiple files and is my biggest
>           pet-peve about the tool.

Have you ever tried pressing Shift + i on the change screen?

>         - patch series are a pain to apply, I have to manually open each
>           patch, give it a +2 and then when all of them are reviewed and
>           past testing, then they will be merged.  Is a pain.  Does no
>           one else accept patch series of 10-30 patches at a time other
>           than me?  How do they do it without opening 30+ tabs?

How would you want it to work otherwise? I assume that you are
actually looking at each patch, and would want to record the fact
they're OK with a +2.

If you're the boss of the project (like in your mbox scenario below)
and are in charge of the gerrit instance, you can remove all the
review requirements, but restrict merge privileges to yourself. You'd
have a "merge including parents" button on the tip of the patch series
without any voting requirements.

> And, by reference of the "slow" issue, I should not have to do multiple
> round-trip queries of a web interface just to see a single patch.
> There's the initial cover page, then there's a click on each individual
> file, bring up a new page for each and every file that was changed for
> that commit to read it, and then when finished, clicking again to go
> back to the initial page, and then a click to give a +2 and another
> click to give a verified and then another refresh of the whole thing.

The 'verified' label was meant for automation systems. If you click
the 'verified' label yourself, that suggests that there is no test
automation for this project, and maybe you should just ask the admin
to disable this label.

> When you are reviewing thousands of patches a year, time matters.
> Gerrit just does not cut it at all.  Remember, we only accept 1/3 of the
> patches sent to us.  We are accepting 9 patches an hour, 24 hours a day.
> That means we reject 18 patches an hour at that same time.


I'm curious how you come to this number. When I look at Linus' tree.

git show --no-merges
43b815c6a8e7dbccb5b8bd9c4b099c24bc22d135..8e0d0ad206f08506c893326ca7c9c3d9cc042cef
| grep ^Date | wc

This range is two recent merge commits by Linus about 232 hours apart.
During that window, 386 non-merge change were merged, ie. about 1.6
commit/hour.

> And then there's the issue of access when you do not have internet, like
> I currently do not right now on a plane.  Or a very slow connection.  I
> can still suck down patches in email apply them, and push them out.
> Using gerrit on this connection is impossible.

there is actually a TTY interface to gerrit that has a local cache,
but i'll admit I have never tried it.

> > Building a review tool is not all that easy to do well; by using
> > Gerrit, you get a tool that already exists, works, and has significant
> > corporate support. We at Google have ~11 SWEs working on Gerrit
> > full-time, for example, and we have support from UX research and UI
> > design. The amount of work to tweak Gerrit for Linux kernel
> > development surely is much less than building something from scratch.
>
> I would love to see a better working gerrit today, for google, for the
> developers there as that would save them time and energy that they
> currently waste using it.  But for a distributed development / review
> environment, with multiple people having multiple trees all over the
> place, I don't know how Gerrit would work, unless it is trivial to
> host/manage locally.
>
> > Gerrit has a patchset oriented workflow (where changes are amended all
> > the time), which is a good fit to the kernel's development process.
>
> Maybe for some tiny subsystem's workflows, but not for any with a real
> amount of development.  Try dumping a subsystem's patches into gerrit
> today.  Can it handle something like netdev?  linux-input?  linux-usb?
> staging?  Where does it start to break down in just being able to handle
> the large quantities of changes?  Patchwork has done a lot to help some

I am not sure I understand the question. We merge about 300 commits
per day into https://chromium.googlesource.com/chromium/src/, which
looks to be much more than what the linux kernel receives.

What do you mean with 'break down' ?

> > There is talk of building a distributed/federated tool, but if there
> > are policies ("Jane Doe is maintainer of the network subsystem, and
> > can merge changes that only touch file in net/ "), then building
> > something decentralized is really hard. You have to build
> > infrastructure where Jane can prove to others who she is (PGP key
> > signing parties?), and some sort of distributed storage of the policy
> > rules.
> >
> > By contrast, a centralized server can authenticate users reliably and
> > the server owner can define such rules.  There can still be multiple
> > gerrit servers, possibly sponsored by corporate entities (one from
> > RedHat, one from Google, etc.), and different servers can support
> > different authentication models (OpenID, OAuth, Google account, etc.)
>
> Like Daniel said, the kernel is multi-repos for a mono-tree.  I don't
> think Gerrit is set up to handle that at all from what I can see.

What problem is solved by having multiple copies of the same tree?

The normal Gerrit approach would be to have one server, with one Linux
repository, with multiple branches. Gerrit has per-branch permissions,
so you can define custom permissions (eg. merge, vote, read, amend
other people's changes) for different branches. Using branches in this
way would allow you to retarget a change to another branch without
losing the review history, or cherry-picking a change to another
branch from within the UI.

> How many people does it take to maintain an Gerrit instance and keep it
> up and running well?

To be honest, I don't know exactly, as the Google setup is very different.

Anecdotally, I hear that it is a setup and forget type of server,
except for upgrades, which require an hour or so of downtime.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

  parent reply	other threads:[~2019-10-15 18:58 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-24 18:25 thoughts on a Merge Request based development workflow Neil Horman
2019-09-24 18:37 ` Drew DeVault
2019-09-24 18:53   ` Neil Horman
2019-09-24 20:24     ` Laurent Pinchart
2019-09-24 22:25       ` Neil Horman
2019-09-25 20:50         ` Laurent Pinchart
2019-09-25 21:54           ` Neil Horman
2019-09-26  0:40           ` Neil Horman
2019-09-28 22:58             ` Steven Rostedt
2019-09-28 23:16               ` Dave Airlie
2019-09-28 23:52                 ` Steven Rostedt
2019-10-01  3:22                 ` Daniel Axtens
2019-10-01 21:14                   ` Bjorn Helgaas
2019-09-29 11:57               ` Neil Horman
2019-09-29 12:55                 ` Dmitry Vyukov
2019-09-30  1:00                   ` Neil Horman
2019-09-30  6:05                     ` Dmitry Vyukov
2019-09-30 12:55                       ` Neil Horman
2019-09-30 13:20                         ` Nicolas Belouin
2019-09-30 13:40                         ` Dmitry Vyukov
2019-09-30 21:02                     ` Konstantin Ryabitsev
2019-09-30 14:51                   ` Theodore Y. Ts'o
2019-09-30 15:15                     ` Steven Rostedt
2019-09-30 16:09                       ` Geert Uytterhoeven
2019-09-30 20:56                       ` Konstantin Ryabitsev
2019-10-08  1:00                     ` Stephen Rothwell
2019-09-26 10:23           ` Geert Uytterhoeven
2019-09-26 13:43             ` Neil Horman
2019-10-07 15:33   ` David Miller
2019-10-07 15:35     ` Drew DeVault
2019-10-07 16:20       ` Neil Horman
2019-10-07 16:24         ` Drew DeVault
2019-10-07 18:43           ` David Miller
2019-10-07 19:24             ` Eric Wong
2019-10-07 15:47     ` Steven Rostedt
2019-10-07 18:40       ` David Miller
2019-10-07 18:45       ` David Miller
2019-10-07 19:21         ` Steven Rostedt
2019-10-07 21:49     ` Theodore Y. Ts'o
2019-10-07 23:00     ` Daniel Axtens
2019-10-08  0:39       ` Eric Wong
2019-10-08  1:26         ` Daniel Axtens
2019-10-08  2:11           ` Eric Wong
2019-10-08  3:24             ` Daniel Axtens
2019-10-08  6:03               ` Eric Wong
2019-10-08 10:06                 ` Daniel Axtens
2019-10-08 13:19                   ` Steven Rostedt
2019-10-08 18:46                 ` Rob Herring
2019-10-08 21:36                   ` Eric Wong
2019-10-08  1:17       ` Steven Rostedt
2019-10-08 16:43         ` Don Zickus
2019-10-08 17:17           ` Steven Rostedt
2019-10-08 17:39             ` Don Zickus
2019-10-08 19:05               ` Konstantin Ryabitsev
2019-10-08 20:32                 ` Don Zickus
2019-10-08 21:35                   ` Konstantin Ryabitsev
2019-10-09 21:50                     ` Laura Abbott
2019-10-10 12:48                       ` Neil Horman
2019-10-09 21:35                 ` Laura Abbott
2019-10-09 21:54                   ` Konstantin Ryabitsev
2019-10-09 22:09                     ` Laura Abbott
2019-10-09 22:19                       ` Dave Airlie
2019-10-09 22:21                     ` Eric Wong
2019-10-09 23:56                       ` Konstantin Ryabitsev
2019-10-10  0:07                         ` Eric Wong
2019-10-10  7:35                         ` Nicolas Belouin
2019-10-10 12:53                           ` Steven Rostedt
2019-10-10 14:21                           ` Dmitry Vyukov
2019-10-11  7:12                             ` Nicolas Belouin
2019-10-11 13:56                               ` Dmitry Vyukov
2019-10-14  7:31                                 ` Nicolas Belouin
2019-10-10 17:52                     ` Dmitry Vyukov
2019-10-10 20:57                       ` Theodore Y. Ts'o
2019-10-11 11:01                         ` Dmitry Vyukov
2019-10-11 12:54                           ` Theodore Y. Ts'o
2019-10-14 19:08                         ` Han-Wen Nienhuys
2019-10-15  1:54                           ` Theodore Y. Ts'o
2019-10-15 12:00                             ` Daniel Vetter
2019-10-15 13:14                               ` Han-Wen Nienhuys
2019-10-15 13:45                                 ` Daniel Vetter
2019-10-16 18:56                                   ` Han-Wen Nienhuys
2019-10-16 19:08                                     ` Mark Brown
2019-10-17 10:22                                       ` Han-Wen Nienhuys
2019-10-17 11:24                                         ` Mark Brown
2019-10-17 11:49                                     ` Daniel Vetter
2019-10-17 12:09                                       ` Han-Wen Nienhuys
2019-10-17 12:53                                         ` Daniel Vetter
2019-10-15 16:07                           ` Greg KH
2019-10-15 16:35                             ` Steven Rostedt
2019-10-15 18:58                             ` Han-Wen Nienhuys [this message]
2019-10-15 19:33                               ` Greg KH
2019-10-15 20:03                                 ` Mark Brown
2019-10-15 19:50                               ` Mark Brown
2019-10-15 18:37                           ` Konstantin Ryabitsev
2019-10-15 19:15                             ` Han-Wen Nienhuys
2019-10-15 19:35                               ` Greg KH
2019-10-15 19:41                               ` Konstantin Ryabitsev
2019-10-16 18:33                                 ` Han-Wen Nienhuys
2019-10-09  2:02           ` Daniel Axtens
2019-09-24 23:15 ` David Rientjes
2019-09-25  6:35   ` Toke Høiland-Jørgensen
2019-09-25 10:49   ` Neil Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFQ2z_N0Hriv68HajKZoNg1AyXzd_GqT8a2sDoCRZkUJy_vKSA@mail.gmail.com \
    --to=hanwen@google.com \
    --cc=davem@davemloft.net \
    --cc=dja@axtens.net \
    --cc=dvyukov@google.com \
    --cc=dzickus@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=konstantin@linuxfoundation.org \
    --cc=labbott@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=rostedt@goodmis.org \
    --cc=sir@cmpwn.com \
    --cc=tytso@mit.edu \
    --cc=workflows@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.