From: Jeff King <email@example.com>
To: Derrick Stolee <firstname.lastname@example.org>
Cc: Ben Humphreys <email@example.com>,
Junio C Hamano <firstname.lastname@example.org>,
Christopher Schenk <email@example.com>,
Subject: Re: [PATCH 2/2] Revert "remote-curl: fall back to basic auth if Negotiate fails"
Date: Wed, 19 May 2021 10:14:28 -0400 [thread overview]
Message-ID: <YKUdRNH+uLBxAifirstname.lastname@example.org> (raw)
On Wed, May 19, 2021 at 09:58:50AM -0400, Derrick Stolee wrote:
> > (Note that this isn't a pure revert; the previous commit added a test
> > showing the regression, so we can now flip it to expect_success).
> Keeping the test is excellent, because it gives us a way to confirm
> that a second attempt at a fix is at least as good as the first.
> The only thing that could improve this situation is to add a test
> that checks the bug that the previous version introduced, so that
> the next round doesn't repeat the mistake. That can be deferred
> because it is more important that we get this fix in time for the
> next release candidate.
Re-reading what I wrote, I think "the previous commit" may be ambiguous.
The original commit which introduced the bug (and which we're reverting
here) didn't include a test at all. In patch 1/2 of this series (what
I'm calling "the previous commit"), I provided a test which shows the
regression. And now this revert shows that we fixed it (by flipping from
expect_failure to expect_success).
So I think I've already done what you're asking (if I understand it
It probably would be a little easier to follow by reverting first, and
then adding in the expect_success test on top to future-proof us. But by
doing it in the other order, it was easy to see the test demonstrate the
behavior before and after the revert.
next prev parent reply other threads:[~2021-05-19 14:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-18 3:07 Git 2.23.0-rc0 HTTP authentication failure - error message change Ben Humphreys
2021-05-18 5:50 ` Jeff King
2021-05-18 6:26 ` [PATCH 0/2] fix v2.32.0-rc0 bug with Negotiate / HTTP_NOAUTH Jeff King
2021-05-18 6:27 ` [PATCH 1/2] t5551: test http interaction with credential helpers Jeff King
2021-05-18 6:27 ` [PATCH 2/2] Revert "remote-curl: fall back to basic auth if Negotiate fails" Jeff King
2021-05-19 13:58 ` Derrick Stolee
2021-05-19 14:14 ` Jeff King [this message]
2021-05-19 14:53 ` Derrick Stolee
2021-05-19 1:12 ` [PATCH 0/2] fix v2.32.0-rc0 bug with Negotiate / HTTP_NOAUTH Junio C Hamano
2021-05-19 0:12 ` Git 2.23.0-rc0 HTTP authentication failure - error message change brian m. carlson
2021-05-19 11:49 ` Jeff King
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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).