git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Jonathan Tan" <jonathantanmy@google.com>,
	git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: What's cooking in git.git (Mar 2018, #03; Wed, 14)
Date: Fri, 16 Mar 2018 17:53:39 -0700	[thread overview]
Message-ID: <xmqq1sgjzfn0.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180316220824.GC151588@aiede.svl.corp.google.com> (Jonathan Nieder's message of "Fri, 16 Mar 2018 15:08:24 -0700")

Jonathan Nieder <jrnieder@gmail.com> writes:

>> I was hoping to hear quick Acks for remove-ignore-env (and also
>> Duy's reroll of their topics on it) from people involved in all the
>> related topics, so that we can advance them more-or-less at the same
>> time.
>
> I can go along with this for this series, but I want to explain why I
> am not thrilled with the process in general.
>
> The series "Moving global state into the repository object (part 1)"
> had gone through three revisions with relatively minor changes between
> each and we were in the middle of reviewing the fourth.  In response
> to a series that comes after it, "(part 2)", Duy created
> nd/remove-ignore-env, a series that I quite like.
>
> But with this reroll, the result is:
> ...
> So even though I agree with the goal and I like the improved
> initialization code, I am not happy.

Well, I do not think it was the "process" issue in particular,
though.  Yes, it was unfortunate that these three revisions were
reviewed but they each resulted with only relative minor changes
between iterations.  IOW, reviewers that lost fresh eyes, perhaps
including me, all missed a larger wart in the design that made the
"ignore-env" thing required in the structure, which was spotted by
somebody else.  It would have been nicer if such an improvement were
done during an earlier part of the cycle and did not require that
other reviewer.

But we didn't spot it, and now we know a better structure of the
topic, luckily before all the things touch 'next'.  We not just got
a suggestion "it would be better to rewrite it like so---now go
ahead to do that", we got something even better and Duy did all the
work restructuring the series, reshuffling the patches.  We should
be thankful, not unhappy.


      reply	other threads:[~2018-03-17  0:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-15  1:34 Junio C Hamano
2018-03-15  6:30 ` Duy Nguyen
2018-03-15 16:54   ` Junio C Hamano
2018-03-15  8:36 ` Ævar Arnfjörð Bjarmason
2018-03-15 17:33   ` Junio C Hamano
2018-03-19 21:16   ` Derrick Stolee
2018-03-15 19:18 ` Lars Schneider
2018-03-15 23:00   ` Lars Schneider
2018-03-16  0:54   ` Junio C Hamano
2018-03-16 21:31 ` Jonathan Tan
2018-03-16 21:52   ` Junio C Hamano
2018-03-16 22:08     ` Jonathan Nieder
2018-03-17  0:53       ` Junio C Hamano [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq1sgjzfn0.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=pclouds@gmail.com \
    --subject='Re: What'\''s cooking in git.git (Mar 2018, #03; Wed, 14)' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).