git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Michael Haggerty" <mhagger@alum.mit.edu>,
	"Eric Wong" <normalperson@yhbt.net>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: Our cumbersome mailing list workflow
Date: Tue, 2 Dec 2014 19:53:47 -0800	[thread overview]
Message-ID: <20141203035347.GH6527@google.com> (raw)
In-Reply-To: <CAGZ79kagELCSkZ0CA1A7gc7CifjToYmb4kiBYQCse3Q7Hwca5Q@mail.gmail.com>

Stefan Beller wrote:

> How are non-regulars/newcomers, who supposingly need more iterations on
> a patch, supposed to handle the inter patch change log conveniently?

I think this is one of the more important issues.

I don't think there's any reason that newcomers should need more
iterations than regulars to finish a patch.  Regulars are actually
held to a higher standard, so they are likely to need more iterations.

A common mistake for newcomers, that I haven't learned yet how to warn
properly against, is to keep re-sending minor iterations on a patch
too quickly.  Some ways to avoid that:

 * feel free to respond to review comments with something like "how
   about this?" and a copy/pasted block of code that just addresses
   that one comment.  That way, you can clear up ambiguity and avoid
   the work of applying that change to the entire patch if it ends
   up seeming like a bad idea.  This also avoids having to re-send a
   larger patch or series multiple times to clear up a small ambiguity
   from a review.

 * be proactive.  Look for other examples of the same issue that a
   reviewer pointed out once so they don't have to find it again
   elsewhere in the next iteration.  Run the testsuite.  Build with
   the flags from
   https://kernel.googlesource.com/pub/scm/git/git/+/todo/Make#106
   in CFLAGS in config.mak.  Proofread and try to read as though you
   knew nothing about the patch to anticipate what reviewers will
   find.

 * feel free to get more review out-of-band, too.  If you're still
   playing with ideas and want someone to take a quick glance before
   the patches are in reviewable form, you can do that and say so
   (e.g., with 'RFC/' before 'PATCH' in the subject line).

Jonathan

  reply	other threads:[~2014-12-03  3:53 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18 22:43 [PATCH] refs.c: use a stringlist for repack_without_refs Stefan Beller
2014-11-18 23:06 ` Junio C Hamano
2014-11-18 23:39   ` Junio C Hamano
2014-11-18 23:45 ` Jonathan Nieder
2014-11-19  0:28   ` Stefan Beller
2014-11-19  1:08   ` Stefan Beller
2014-11-19 18:00     ` Junio C Hamano
2014-11-19 18:50       ` Stefan Beller
2014-11-19 20:44         ` Jonathan Nieder
2014-11-19 21:54           ` Stefan Beller
2014-11-19 21:59             ` [PATCH v4] " Stefan Beller
2014-11-20  2:15               ` Jonathan Nieder
2014-11-20 16:47                 ` Junio C Hamano
2014-11-20 18:04                 ` [PATCH v5 1/1] " Stefan Beller
2014-11-20 18:10                   ` [PATCH] refs.c: repack_without_refs may be called without error string buffer Stefan Beller
2014-11-20 18:15                     ` Ronnie Sahlberg
2014-11-20 18:35                     ` Jonathan Nieder
2014-11-20 18:36                       ` Ronnie Sahlberg
2014-11-20 18:56                         ` Stefan Beller
2014-11-20 18:29                   ` [PATCH v5 1/1] refs.c: use a stringlist for repack_without_refs Jonathan Nieder
2014-11-20 18:37                   ` Jonathan Nieder
2014-11-20 19:01                   ` Junio C Hamano
2014-11-20 19:05                     ` Stefan Beller
2014-11-20 20:07                       ` [PATCH v6] refs.c: use a string_list " Stefan Beller
2014-11-20 20:36                         ` Jonathan Nieder
2014-11-21 14:09         ` [PATCH 0/6] repack_without_refs(): convert to string_list Michael Haggerty
2014-11-21 14:09           ` [PATCH 1/6] prune_remote(): exit early if there are no stale references Michael Haggerty
2014-11-22 21:07             ` Jonathan Nieder
2014-11-21 14:09           ` [PATCH 2/6] prune_remote(): initialize both delete_refs lists in a single loop Michael Haggerty
2014-11-21 14:09           ` [PATCH 3/6] prune_remote(): sort delete_refs_list references en masse Michael Haggerty
2014-11-21 16:44             ` Junio C Hamano
2014-11-25  7:21               ` Michael Haggerty
2014-11-25  8:04                 ` Michael Haggerty
2014-11-22 21:08             ` Jonathan Nieder
2014-11-21 14:09           ` [PATCH 4/6] repack_without_refs(): make the refnames argument a string_list Michael Haggerty
2014-11-22 21:17             ` Jonathan Nieder
2014-11-25  7:42               ` Michael Haggerty
2014-11-21 14:09           ` [PATCH 5/6] prune_remote(): rename local variable Michael Haggerty
2014-11-22 21:18             ` Jonathan Nieder
2014-11-21 14:09           ` [PATCH 6/6] prune_remote(): iterate using for_each_string_list_item() Michael Haggerty
2014-11-22 21:19             ` Jonathan Nieder
2014-11-21 14:25           ` [PATCH 0/6] repack_without_refs(): convert to string_list Michael Haggerty
2014-11-21 18:00             ` Junio C Hamano
2014-11-21 19:57               ` Stefan Beller
2014-11-25  0:28               ` Our cumbersome mailing list workflow (was: Re: [PATCH 0/6] repack_without_refs(): convert to string_list) Michael Haggerty
2014-11-27 17:46                 ` Our cumbersome mailing list workflow Torsten Bögershausen
2014-11-27 18:24                   ` Matthieu Moy
2014-11-28 12:09                     ` Philip Oakley
2014-11-27 22:53                   ` Eric Wong
2014-11-28 15:34                     ` Michael Haggerty
2014-11-28 16:24                       ` brian m. carlson
2014-12-01  2:46                       ` Junio C Hamano
2014-12-03  2:20                         ` Stefan Beller
2014-12-03  3:53                           ` Jonathan Nieder [this message]
2014-12-03 17:18                             ` Junio C Hamano
2014-12-03 17:28                           ` Torsten Bögershausen
2014-11-28 14:31                   ` Michael Haggerty
2014-11-28 15:42                     ` Marc Branchaud
2014-11-28 21:39                       ` Damien Robert
2014-12-03 23:57                 ` Philip Oakley
2014-12-04  2:03                   ` Stefan Beller

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=20141203035347.GH6527@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=normalperson@yhbt.net \
    --cc=sbeller@google.com \
    --cc=tboegi@web.de \
    /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;
as well as URLs for NNTP newsgroup(s).