Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Pushkar Singh <pushkarkumarsingh1970@gmail.com>,
	git@vger.kernel.org, peff@peff.net, r.norouzi@proton.me
Subject: Re: [PATCH v3] reflog: fix default expiry periods
Date: Tue, 29 Sep 2026 07:45:41 +0200	[thread overview]
Message-ID: <artQhZKf6JuRhmRl@pks.im> (raw)
In-Reply-To: <xmqqy0clo2em.fsf@gitster.g>

On Mon, Sep 28, 2026 at 07:50:57AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Thu, Sep 24, 2026 at 05:58:44PM +0000, Pushkar Singh wrote:
> >> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> >> index 8f78cf4b01..93b5b49e1d 100755
> >> --- a/t/t1410-reflog.sh
> >> +++ b/t/t1410-reflog.sh
> >> @@ -153,6 +153,72 @@ test_expect_success 'reflog expire should not barf on an annotated tag' '
> >>  	test_grep ! "error: [Oo]bject .* not a commit" err
> >>  '
> >>  
> >> +test_expect_success 'reflog expire keeps reachable entries for 90 days' '
> >> +	test_when_finished "rm -rf reachable-keep" &&
> >> +	git init reachable-keep &&
> >> +	(
> >> +		cd reachable-keep &&
> >> +		timestamp=$(test-tool date timestamp "60.days.ago") &&
> >
> > Nit: I would've preferred to make this 89 days...
> 
> Dates calculated as 89 days ago from the beginning of today, from
> the end of today, and from this very minute can differ by almost 24
> hours.  Because we are not interested in testing what semantics
> approxidate() implements in test-tool date timestamp, but are
> testing what expiry period reflog expire implements between 30 and
> 90 days, using numbers that are not too close to the edge spares us
> from having to worry about boundary cases we do not care about.
> 
> So I wouldn't have preferred using 89 days there.

Fair enough. I just find it a bit fishy to assert that we "[keep]
reachable entries for 90 days" by checking that we keep it for 60 days
but throw it away after 100 days. THat allows for a very wide range of
values that aren't 90 days.

So even if it shouldn't be 89 days, it could very well have been 88 days
without any risk for test flakiness.

Patrick

  reply	other threads:[~2026-09-29  5:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 18:32 reflog expire: default expiry times swapped since 2.50 r.norouzi
2026-09-22 16:54 ` [PATCH] reflog: fix default expiry periods Pushkar Singh
2026-09-22 17:27   ` Junio C Hamano
2026-09-22 18:02     ` Jeff King
2026-09-23 12:21       ` Patrick Steinhardt
2026-09-23 10:21   ` [PATCH v2] " Pushkar Singh
2026-09-23 19:26     ` Junio C Hamano
2026-09-24 14:12       ` Patrick Steinhardt
2026-09-24 15:46         ` Jeff King
2026-09-24 17:45           ` Junio C Hamano
2026-09-24 18:43             ` Jeff King
2026-09-28  6:53               ` Patrick Steinhardt
2026-09-24 17:43         ` Junio C Hamano
2026-09-24 17:58     ` [PATCH v3] " Pushkar Singh
2026-09-24 18:26       ` Junio C Hamano
2026-09-28  7:02       ` Patrick Steinhardt
2026-09-28 14:50         ` Junio C Hamano
2026-09-29  5:45           ` Patrick Steinhardt [this message]
2026-09-29 18:31             ` Junio C Hamano

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=artQhZKf6JuRhmRl@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=pushkarkumarsingh1970@gmail.com \
    --cc=r.norouzi@proton.me \
    /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