Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Subject: some thoughts on gerrit
Date: Sun, 20 Oct 2019 14:37:00 -0000	[thread overview]
Message-ID: <877e4z8ovc.fsf@tromey.com> (raw)

After using gerrit for gdb for just this past week, I have mixed
feelings, which I thought I'd share.

The upside is pretty good -- basically what I was hoping for when we
discussed this topic at Cauldron.

The major benefits is that it's easy to see the status of patches.

A benefit I didn't predict is that it's a bit simpler to submit patches.
In particular, my personal email host doesn't like it if I send log
series, so I have to remember to throttle when using send-email -- but
with gerrit that's a non-issue, because it is just a push.


So far the major downsides are related to patch series.

[[[
First, as an aside, my most recent patch series does show up here:

https://sourceware.org/ml/gdb-patches/2019-10/authors.html

(Search for "RAII class" under the name "Tom Tromey (Code Review)")

...but it somehow doesn't show up here:

https://sourceware.org/ml/gdb-patches/2019-10/threads.html
]]]


Anyway, with series we are missing two things that email had: namely,
the cover letter, and threading.

The cover letter is often a good guide to what is going on in the
series.  See for example:

https://sourceware.org/ml/gdb-patches/2019-10/msg00590.html

It seems a shame to lose this.

One counter-argument about the cover letter I thought of is that,
because it doesn't end up in the history, maybe the lack of it will
force us to make each commit message even clearer.  And, we should do
that ... it's just that the roadmap is handy when reviewing.

I wonder if there'd be a way to make "git review" prompt for a cover
letter and attach it somewhere as a comment.


Lack of threading means it is hard to see how patches relate when
reading in email.  Maybe this can be fixed in gerrit?


I wonder a little if a sufficiently configured patchworks would be a
better fit for gdb.

The major problem with the current patchworks is that it doesn't
automatically remove patches when they are checked in.  However, a newer
version of patchworks can do this, especially if we augment it with a
commit hook to add a UUID to the commit message (which we've already
accepted for gerrit...).  It seems easy to set this up.

Another drawback of patchworks is that reviews aren't done online -- you
still use email.  This doesn't bother me, but maybe it's an important
consideration for others.

I'm not saying we should definitely switch -- just that I've noticed
some drawbacks to gerrit as compared to email, and I am wondering if we
can somehow preserve more of the good things.

Tom


             reply	other threads:[~2019-10-20 14:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-20 14:37 Tom Tromey [this message]
2019-10-20 22:25 ` Sergio Durigan Junior
2019-10-20 22:30   ` Sergio Durigan Junior
2019-10-21  9:02   ` Andreas Schwab
2019-10-21 20:59     ` Christian Biesinger via gdb-patches
2019-10-21 21:24       ` Sergio Durigan Junior
2019-10-21 21:25     ` Sergio Durigan Junior
2019-10-20 23:17 ` Andrew Pinski
2019-10-21  1:49   ` Simon Marchi
2019-10-21  3:26 ` Simon Marchi

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=877e4z8ovc.fsf@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox