git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Eric Wong <e@80x24.org>, Jeff King <peff@peff.net>,
	Dan Wang <dwwang@google.com>,
	Dennis Kaarsemaker <dennis@kaarsemaker.net>
Subject: Re: [PATCH 1/4] push options: {pre,post}-receive hook learns about push options
Date: Thu, 7 Jul 2016 14:50:55 -0700	[thread overview]
Message-ID: <CAGZ79kb_oHZUv1=8Q0FSV8yVP5PrwStchXAyhZF=mHhv91APHQ@mail.gmail.com> (raw)
In-Reply-To: <xmqqeg75p5cn.fsf@gitster.mtv.corp.google.com>

On Thu, Jul 7, 2016 at 1:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> What is suboptimal about the structure of the series is that we
> won't bisect down to any of the potential bugs in the above code
> even if we ever see any bug in the future.
> It also does not hint
> where push_options is expected to be read in the code in the
> subsequent patches in the series.  If I were doing this series, I
> would probably have done 2/4 first without plumbing it through
> (i.e. it is sent and accumulated in a string list at the receiver,
> and then cleared and freed without being used), and then added the
> processing (i.e. this step) as the second patch.

But your first patch (2/4) would not yet advertise the capability?
Or advertise and then just ignoring it?

That shadows other bugs that would not properly bisect, I'd imagine?

It is better for documentation purposes in this patch though. It makes
the other patch harder as "it allows transmitting push options, but
in that patch nothing of value is done with them."

So I'll see if I can reorder easily.

Thanks,
Stefan

  reply	other threads:[~2016-07-07 21:50 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-07  1:12 [PATCHv3 0/4] Push options in C Git Stefan Beller
2016-07-07  1:12 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-07 20:20   ` Junio C Hamano
2016-07-07 21:50     ` Stefan Beller [this message]
2016-07-07 21:53       ` Junio C Hamano
2016-07-07  1:12 ` [PATCH 2/4] receive-pack: implement advertising and receiving " Stefan Beller
2016-07-07 20:37   ` Junio C Hamano
2016-07-07 21:41     ` Stefan Beller
2016-07-07 21:56       ` Jeff King
2016-07-07 22:06         ` Stefan Beller
2016-07-07 22:09           ` Jeff King
2016-07-07 22:06       ` Junio C Hamano
2016-07-08 17:58         ` Jonathan Nieder
2016-07-08 18:39           ` Junio C Hamano
2016-07-08 18:57             ` Stefan Beller
2016-07-08 21:46               ` Jeff King
2016-07-08 22:17                 ` Stefan Beller
2016-07-08 22:21                   ` Jeff King
2016-07-08 22:29                     ` Stefan Beller
2016-07-08 22:35                       ` Jeff King
2016-07-08 22:43                         ` Stefan Beller
2016-07-08 22:46                           ` Jeff King
2016-07-08 22:51                             ` Stefan Beller
2016-07-07  1:12 ` [PATCH 3/4] push: accept " Stefan Beller
2016-07-07 20:52   ` Junio C Hamano
2016-07-08 22:59     ` Stefan Beller
2016-07-11 18:42       ` Junio C Hamano
2016-07-07  1:12 ` [PATCH 4/4] add a test for " Stefan Beller
2016-07-07 19:51   ` Junio C Hamano
2016-07-07 20:01     ` Junio C Hamano
2016-07-07 21:51       ` Stefan Beller
  -- strict thread matches above, loose matches on Subject: below --
2016-07-14 21:49 [PATCHv7 0/4] Push options Stefan Beller
2016-07-14 21:49 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-14 22:46   ` Jeff King
2016-07-14 22:51     ` Stefan Beller
2016-07-14 17:39 [PATCHv5 0/4] Push options Stefan Beller
2016-07-14 17:39 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-09  0:31 [PATCHv4 0/4] Push options Stefan Beller
2016-07-09  0:31 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-06-30  0:59 [RFC PATCHv1 0/4] Push options in C Git Stefan Beller
2016-06-30  0:59 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-01  7:14   ` Jeff King
2016-07-01 17:20     ` Stefan Beller
2016-07-01 17:59       ` Jeff King
2016-07-01 18:03         ` Junio C Hamano
2016-07-01 18:11           ` Jeff King
2016-07-01 19:25             ` Junio C Hamano
2016-07-01 19:31               ` Stefan Beller
2016-07-01 19:39               ` Jeff King
2016-07-01 19:50                 ` Stefan Beller
2016-07-01 18:40         ` 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='CAGZ79kb_oHZUv1=8Q0FSV8yVP5PrwStchXAyhZF=mHhv91APHQ@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=dennis@kaarsemaker.net \
    --cc=dwwang@google.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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).