git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Philippe Blain <levraiphilippeblain@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Peter Kaestle <peter.kaestle@nokia.com>,
	Git mailing list <git@vger.kernel.org>,
	Stefan Beller <stefanbeller@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [RFC 1/2] submodules: test for fetch of non-init subsub-repo
Date: Wed, 11 Nov 2020 12:35:53 -0500	[thread overview]
Message-ID: <37DD13D4-FBE4-4DB7-85F5-824E850BA9AE@gmail.com> (raw)
In-Reply-To: <xmqqk0uuct94.fsf@gitster.c.googlers.com>

Hi Junio,

> Le 9 nov. 2020 à 12:52, Junio C Hamano <gitster@pobox.com> a écrit :
> 
> Peter Kaestle <peter.kaestle@nokia.com> writes:
> 
>> This test case triggers a regression, which was introduced by
>> a62387b3fc9f5aeeb04a2db278121d33a9caafa7 in following setup:
> 
> Also, it probably is a better arrangement, after explaining how the
> current system does not work in the log message, to have the code
> fix in the same patch and add test to ensure the bug will stay
> fixed, in a single patch.  That way, you do not have to start with
> expect_failure and then flip the polarity to expect_success, which
> is a horrible style for reviewers to understand the code fix because
> the second "fix" step does not actually show the effect of what got
> fixed in the patch (the test change shows the flip of the polarity
> of the test plus only a few context lines and does not show what
> behaviour change the "fix" causes).

I had learned by browsing the list that this was the preferred way to
submit patches for bug fixes and regressions for this project, but I had
not yet read a good justification as to *why* it was preferred. 

Thanks for spelling it out; I think a quick paragraph about this somewhere
in SubmittingPatches would be a good addition for new contributors.

Cheers,

Philippe.


  parent reply	other threads:[~2020-11-11 17:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-03 14:23 [REGRESSION FIX 0/2] Handling regression introduced by a62387b Peter Kaestle
2020-11-03 14:23 ` [REGRESSION FIX 1/2] submodules: test for fetch of non-init subsub-repo Peter Kaestle
2020-11-03 14:23 ` [REGRESSION FIX 2/2] Revert "submodule.c: fetch in submodules git directory instead of in worktree" Peter Kaestle
2020-11-09  8:33 ` [RFC 0/2] Handling regression introduced by a62387b Peter Kaestle
2020-11-09  8:33   ` [RFC 1/2] submodules: test for fetch of non-init subsub-repo Peter Kaestle
2020-11-09 17:52     ` Junio C Hamano
2020-11-11 12:45       ` Peter Kästle
2020-11-11 17:22         ` Philip Oakley
2020-11-12 16:00           ` [RFCv2] submodules: fix of regression on fetching " Peter Kaestle
2020-11-11 17:35       ` Philippe Blain [this message]
2020-11-11 19:27         ` [RFC 1/2] submodules: test for fetch " Junio C Hamano
2020-11-11 19:24     ` Philippe Blain
2020-11-09  8:33   ` [RFC 2/2] Revert "submodule.c: fetch in submodules git directory instead of in worktree" Peter Kaestle
2020-11-10 15:08     ` Johannes Schindelin

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=37DD13D4-FBE4-4DB7-85F5-824E850BA9AE@gmail.com \
    --to=levraiphilippeblain@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peter.kaestle@nokia.com \
    --cc=stefanbeller@gmail.com \
    /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.
Code repositories for project(s) associated with this public 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).