git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Hendrickson <ryan.hendrickson@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v5] http: do not ignore proxy path
Date: Fri, 2 Aug 2024 17:26:32 -0400 (EDT)	[thread overview]
Message-ID: <c7b3383b-ebbf-6ec4-00b8-41c9430a99f4@alum.mit.edu> (raw)
In-Reply-To: <xmqqfrrmbpbv.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]

At 2024-08-02 14:13-0700, Junio C Hamano <gitster@pobox.com> sent:

>>>>>> Is this blocking feedback? This strikes me as speculative
>>>>>> over-engineering
>>>>>
>>>>> No, it is loosening a pattern that is overly tight and as a side
>>>>> effect shortening the line to more readable length ;-).
>>>>
>>>> Blocking or not?
>>>
>>> If we are updating anyway, that question is irrelevant, no?  This
>>> version may hit 'seen' but until the next version comes it will not
>>> advance to 'next'.
>>
>> I can't figure out what you mean by this so I am going to proceed as
>> if you had simply said ‘non-blocking’.
>
> It does not make much sense to ask if a suggestion is "blocking" or
> "non-blocking".  If you respond with a reasonable explanation why
> you do not want to take a suggestion, I may (or may not) say that
> your reasoning makes sense.  IOW, making me say "it is blocking"
> means you want to me to say that I won't listen to you no matter
> what you say.  That is rarely be the case.

I want you to do what Documentation/ReviewingGuidelines.txt says reviewers 
should do:

- When providing a recommendation, be as clear as possible about whether you
   consider it "blocking" (the code would be broken or otherwise made worse if an
   issue isn't fixed) or "non-blocking" (the patch could be made better by taking
   the recommendation, but acceptance of the series does not require it).
   Non-blocking recommendations can be particularly ambiguous when they are
   related to - but outside the scope of - a series ("nice-to-have"s), or when
   they represent only stylistic differences between the author and reviewer.

Because I would rather not have what is likely to be a highly subjective 
argument about this particular choice in a test script if we don't have 
to have one.

I would also rather not put time into figuring out how best to rename the 
function "old_curl_version" if it no longer checks for the particular 
error produced when the curl version is too old, nor would I want to think 
ahead about whether it is correct for these tests that use this function 
to continue to pass if different variations on this error are raised. 
There is one qualifying error currently, and that's what the test matches 
against. Matching against hypothetical future errors is speculative 
overengineering if it is not obvious how the errors will vary and what it 
may mean if they do.

R

  reply	other threads:[~2024-08-02 21:26 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-26 15:52 [PATCH] http: do not ignore proxy path Ryan Hendrickson via GitGitGadget
2024-07-26 16:29 ` Junio C Hamano
2024-07-26 17:12   ` Ryan Hendrickson
2024-07-26 17:45     ` Junio C Hamano
2024-07-26 21:11 ` Jeff King
2024-07-26 22:43   ` Ryan Hendrickson
2024-07-29 19:31     ` Jeff King
2024-07-27  6:44 ` [PATCH v2] " Ryan Hendrickson via GitGitGadget
2024-07-29 20:09   ` Jeff King
2024-07-31 15:33     ` Ryan Hendrickson
2024-07-31 16:01   ` [PATCH v3] " Ryan Hendrickson via GitGitGadget
2024-07-31 22:24     ` Junio C Hamano
2024-08-01  3:44       ` Ryan Hendrickson
2024-08-01  5:21         ` Junio C Hamano
2024-08-01  5:45       ` Jeff King
2024-08-01 14:40         ` Junio C Hamano
2024-08-01  5:22     ` [PATCH v4] " Ryan Hendrickson via GitGitGadget
2024-08-01  6:04       ` Jeff King
2024-08-01 17:04         ` Junio C Hamano
2024-08-02  5:20       ` [PATCH v5] " Ryan Hendrickson via GitGitGadget
2024-08-02 15:52         ` Junio C Hamano
2024-08-02 16:43           ` Ryan Hendrickson
2024-08-02 17:10             ` Junio C Hamano
2024-08-02 18:03               ` Ryan Hendrickson
2024-08-02 19:28                 ` Junio C Hamano
2024-08-02 19:39                   ` Ryan Hendrickson
2024-08-02 21:13                     ` Junio C Hamano
2024-08-02 21:26                       ` Ryan Hendrickson [this message]
2024-08-02 21:43                         ` Junio C Hamano
2024-08-02 21:47                         ` Junio C Hamano
2024-08-02 22:14                           ` Ryan Hendrickson

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=c7b3383b-ebbf-6ec4-00b8-41c9430a99f4@alum.mit.edu \
    --to=ryan.hendrickson@alum.mit.edu \
    --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).