All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dorine Tipo <dorine.a.tipo@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, kaartic.sivaraam@gmail.com,
	christian.couder@gmail.com, siddharthasthana31@gmail.com,
	usmanakinyemi202@gmail.com
Subject: Re: [Outreachy PATCH 1/2] t0410: use rev-parse to get the subtree OID
Date: Thu, 15 Oct 2026 18:47:44 +0300	[thread overview]
Message-ID: <atD1oFyLb0k4oFWN@Stealth> (raw)
In-Reply-To: <xmqqcxtcqcfa.fsf@gitster.g>

On Wed, Oct 14, 2026 at 01:01:29PM -0700, Junio C Hamano wrote:
> 
> [devil's advocate hat on]
> 
> That may avoid piping, but are they chekcing the same thing?  If
> HEAD had a tree that sorts before a_dir due to a bug, the original
> test would have used that tree instead of the tree of a_dir.  Later
> tests may have noticed that the tree object is not what is expected
> for a_dir and led to the discovery of such a bug.  The new test goes
> directly to a_dir so it wouldn't catch such a bug.
> 
> [devil's advoicate hat off]
> 
> Arguably, the use of "head -1" in the original pipeline is
> unnecessary as 'repo' prepared there only has a_dir/something and
> nothing else, and if the extra tree that was added by a bug sorted
> after a_dir, it would have hidden such a bug, instead of catching
> it.  So in the sense, it would be better to assume that the tree
> would be created in a shape that we expect and explicitly name the
> directory we are interested in i.e., "HEAD:a_dir".
> 
> So, one thing we may want to add to the second paragraph of the
> proposed log message is that the original assumed silently that the
> first tree object in ls-tree output is the tree we are intrerested
> in, without even validating what path the tree was stored at, and it
> would be an improvement to name 'a_dir' explicitly in the test.
> 
> 
Thank you, that makes sense. I'll add that to the second paragraph in v2.
I'll wait a little to see if any changes are needed on patch 2/2, then
send v2 with everything together.

  reply	other threads:[~2026-10-15 15:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 13:45 [Outreachy PATCH 0/2] t0410: fix exit code and hash assumptions in the missing-tree test Dorine Tipo
2026-10-14 13:45 ` [Outreachy PATCH 1/2] t0410: use rev-parse to get the subtree OID Dorine Tipo
2026-10-14 20:01   ` Junio C Hamano
2026-10-15 15:47     ` Dorine Tipo [this message]
2026-10-14 13:45 ` [Outreachy PATCH 2/2] t0410: keep git's exit code in test_grep calls Dorine Tipo

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=atD1oFyLb0k4oFWN@Stealth \
    --to=dorine.a.tipo@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=siddharthasthana31@gmail.com \
    --cc=usmanakinyemi202@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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.